It has long irritated me that the beforeSaveHandler passes text with embedded metadata to plugins, and then promptly throws the modified metadata away. I cannot work out if this is intentional or not, because as usual the Cairo code is horrible and there are no explanatory comments, but I have to assume that it is a coding error, so I'm registering it as a bug. The plugin should be able to modify meta-data and expect it to be retained in the saved text.

If the handler is fixed there is a small risk that someone has written a plugin that accidentally munges metadata, but it currently works because of the bug. However I think this should be a case of caveat emptor. So rather than adding a new handler in Dakar that works correctly, I am proposing to fix the existing handler.

I have checked all the plugins in the plugins web and examined the way they use this handler, and I can't see any that would have a problem with the fix.

-- CrawfordCurrie - 18 Jun 2005

Fixed in SVN 4384 - CC

-- CrawfordCurrie - 18 Jun 2005

I changed the fix so it preserves the old semantic. Additionally, the meta object is passed to the plugins in the beforeSaveHandler and afterSaveHandler calls. See the documentation of EmptyPlugin for more info.

up in SVN 4405

-- RafaelAlvarez - 20 Jun 2005

I hadn't realised you were going to leave the code that writes and removes the meta-data in the text, so the optimisation (checking for the handler) needed to stay.

On careful reflection, and another review of the plugins checked in to the Plugins web, I think we are justified in changing the semantics. Here's how beforeSaveHandler is used

EmbedTopicPlugin	Global substitute of tags that are never inserted in meta-data
NotifyOnChildModificationPlugin	Text is ignored
MrdIdPlugin	Global substitute of tags
NewsPlugin	Global substitute of tags, process bullet lists
WebDAVPlugin	Processes * Set statements in text
ProgramsPlugin	Global replace over text
TextTogglePlugin	Global replace over text
NotificationPlugin	Ignores text
ApprovalPlugin	Ignores text
NavbarPlugin	Global replace over text
ActionTrackerPlugin	Strips out meta, and puts it back in afterwards.
XmlQueryPlugin	Parses out meta-data and rebuilds the meta-data hash before further processing!

Of all the plugins examined, only XmlQueryPlugin actively processes meta-data, and it would actually be simplified if it used $meta. Most other plugins that global replace over the text are actually more broken by meta-data being present than the converse, because they are not smart enough to differentiate metween meta and plain text.

Given that only one plugin is impacted, I think it is justified to change the semantics, and stop writing the meta-data back into the topic text. If anyone has a problem with this, scream now.

SVN 4406

-- CrawfordCurrie - 21 Jun 2005