Review Request: Use in-memory dom document when editing toolbars, to allow for dynamic changes

[Thomas Friedrichsmeier]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/1694/#review2466
-----------------------------------------------------------


I see, I'm a bit late to reply. But anyway:

I have not really tested this, as I do not generally have the time to follow SVN closely. I do not think this will affect me / my application in any way, but:

In light of this: http://reviewboard.kde.org/r/1238/, I think it would generally be cleaner if XMLGUI-changes are always written to disc somewhere. At least if any third application is expected to embed this client. (The details of the linked patch don't matter too much, here, but to solve the synchronization problems outlined in that review request, hosting applications will probably always rely on reloading the XML from disc one way or another. Alternative: Provide public API to make an XMLGUIClient reload the settings from the localXMLFile, only).

- Thomas


On 2009-09-22 18:38:22, David Faure wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/1694/
> -----------------------------------------------------------
> 
> (Updated 2009-09-22 18:38:22)
> 
> 
> Review request for kdelibs.
> 
> 
> Summary
> -------
> 
> Users of KXmlGuiClient::setDOMDocument probably noticed that the dynamic changes
> made to the guiclient's XML, where not taken into account in KEditToolBar. 
> 
> This is because KEditToolBar loads the XML file again from disk, rather than using
> the in-memory XML tree. (And this in turn is probably because in KDE3, we had
> KXMLGUIClient::conserveMemory which got rid of the in-memory xml tree, but we removed
> it for KDE4 because it broke any kind of dynamic changes like action lists, plugins etc.)
> 
> So the fix is simple: using the in-memory domDocument() instead. The question is what
> it breaks. I noticed that the ui_standards.rc merging left the name and version attributes
> of ui_standards.rc into the toplevel tag -- fixed in r1026838.
> 
> If your app does unusual stuff around xmlguiclients and/or toolbar editing, please
> test this patch before I commit it.
> 
> The reason for all this is bug 207296, but surely I'm not the first user of setDOMDocument.
> I see that knotes, kopete, and kross use it already, at least.
> 
> 
> This addresses bug 207296.
>     https://bugs.kde.org/show_bug.cgi?id=207296
> 
> 
> Diffs
> -----
> 
>   trunk/KDE/kdelibs/kdeui/dialogs/kedittoolbar.cpp 1025321 
> 
> Diff: http://reviewboard.kde.org/r/1694/diff
> 
> 
> Testing
> -------
> 
> Editing toolbars in kontact, konqueror, kate.
> 
> [found an unrelated bug while testing in konq, the Tools menu moves to first position, even w/o the patch...]
> 
> 
> Thanks,
> 
> David
> 
>