xmlgui, DefineGroup vs. MergeLocal

[David Faure]

On Thursday 22 October 2009, Andreas Pakulat wrote:
> On 22.10.09 18:19:09, Andreas Pakulat wrote:
> > On 22.10.09 17:52:49, David Faure wrote:
> > > On Sunday 18 October 2009, Andreas Pakulat wrote:
> > > > So should I go around and add a group for each MergeLocal using
> > > > *_group?
> > >
> > > Yep, I like the idea.
> > > I just hope we don't mess things up for apps that were already defining
> > > groups with such a name though....
> >
> > I was planning to do a test with kdevelop having the same group in two
> > different places (and in the same place) before doing the change.
> 
> Short test indicates that a group definition in ui_standards.rc
> overrides one in an app's ui.rc file. I can try to add a unit-test for
> that to the xmlgui tests.

That would be great. If you do, I promise to fix it.

I think I see the problem already, though: kxmlguiclient::findMatchingElement
matches the two groups with the same name. But then the code assumes that
it's a container (menu/toolbar), because it doesn't know the tag, so it calls 
mergeXML recursively, then notices the container is empty, and deletes it.
But DefineGroup isn't a container ;-)

So maybe this helps?

--- xmlgui/kxmlguiclient.cpp    (revision 1040207)
+++ xmlgui/kxmlguiclient.cpp    (working copy)
@@ -479,6 +479,11 @@

 bool KXMLGUIClientPrivate::isEmptyContainer(const QDomElement& base, KActionCollection *actionCollection) 
const
 {
+    static const QString &tagDefineGroup = KGlobal::staticQString("DefineGroup");
+    if (equalstr(base.tagName(), tagDefineGroup)) {
+        return false; // keep those around, so that KXMLGuiFactory can use them
+    }
+
     // now we check if we are empty (in which case we return "true", to
     // indicate the caller that it can delete "us" (the base element
     // argument of "this" call)

(Hmm, I hope this doesn't prevent you from writing the unit test :-)

-- 
David Faure, faure at kde.org, http://www.davidfaure.fr
Sponsored by Nokia to work on KDE, incl. Konqueror (http://www.konqueror.org).