[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-commits
Subject: KDE/kdelibs/kdeui
From: David Faure <faure () kde ! org>
Date: 2009-03-31 21:53:31
Message-ID: 1238536411.467538.32669.nullmailer () svn ! kde ! org
[Download RAW message or body]
SVN commit 947617 by dfaure:
will Stephenson found an interesting bug in kxmlgui, where <menu name="edit"> \
wouldn't be merged with the uistandards <Menu name="edit">. Tag name comparison has \
to be done case-insensitively, since for some reason xmlgui was made [somewhat] case \
insensitive.
M +13 -0 tests/kxmlgui_unittest.cpp
M +33 -37 xmlgui/kxmlguiclient.cpp
M +1 -1 xmlgui/kxmlguiversionhandler.cpp
--- trunk/KDE/kdelibs/kdeui/tests/kxmlgui_unittest.cpp #947616:947617
@@ -436,6 +436,19 @@
<< (QStringList() << "file_open" << "options_configure_toolbars" << \
"foo_action") << (QStringList() << "file" << "foo" << "settings");
+ QTest::newRow("Bille's testcase: menu patch + menu edit")
+ << xmlBegin + "<Menu name=\"patch\"><Action \
name=\"patch_generate\"/></Menu>\n" + + "<Menu name=\"edit\"><Action \
name=\"edit_foo\"/></Menu>\n" + + xmlEnd
+ << (QStringList() << "file_open" << "patch_generate" << "edit_foo")
+ << (QStringList() << "file" << "edit" << "patch");
+ QTest::newRow("Bille's testcase: menu patch + menu edit, lowercase tag")
+ << xmlBegin + "<Menu name=\"patch\"><Action \
name=\"patch_generate\"/></Menu>\n" + + "<menu name=\"edit\"><Action \
name=\"edit_foo\"/></menu>\n" + + xmlEnd
+ << (QStringList() << "file_open" << "patch_generate" << "edit_foo")
+ << (QStringList() << "file" << "edit" << "patch");
+
// Tests for noMerge="1"
QTest::newRow("noMerge empty file menu, implicit settings menu")
<< xmlBegin + "<Menu name=\"file\" noMerge=\"1\"/>\n" + xmlEnd
--- trunk/KDE/kdelibs/kdeui/xmlgui/kxmlguiclient.cpp #947616:947617
@@ -272,6 +272,11 @@
setXMLGUIBuildDocument( QDomDocument() );
}
+// if (equals(a,b)) is more readable than if (a.compare(b, Qt::CaseInsensitive)==0)
+static bool equals(const QString& a, const QString& b) {
+ return a.compare(b, Qt::CaseInsensitive) == 0;
+}
+
bool KXMLGUIClientPrivate::mergeXML( QDomElement &base, QDomElement &additive, \
KActionCollection *actionCollection ) {
static const QString &tagAction = KGlobal::staticQString( "Action" );
@@ -296,8 +301,6 @@
base.parentNode().replaceChild(additive, base);
base = additive;
} else {
- QString tag;
-
// iterate over all elements in the container (of the global DOM tree)
QDomNode n = base.firstChild();
while ( !n.isNull() )
@@ -307,12 +310,11 @@
if (e.isNull())
continue;
- tag = e.tagName();
+ const QString tag = e.tagName();
// if there's an action tag in the global tree and the action is
// not implemented, then we remove the element
- if ( tag == tagAction )
- {
+ if (equals(tag, tagAction)) {
const QString name = e.attribute(attrName);
if (!actionCollection->action(name) ||
!KAuthorized::authorizeKAction(name))
@@ -325,18 +327,16 @@
// if there's a separator defined in the global tree, then add an
// attribute, specifying that this is a "weak" separator
- else if ( tag == tagSeparator )
- {
+ else if (equals(tag, tagSeparator)) {
e.setAttribute( attrWeakSeparator, (uint)1 );
// okay, hack time. if the last item was a weak separator OR
// this is the first item in a container, then we nuke the
// current one
QDomElement prev = e.previousSibling().toElement();
- if ( prev.isNull() ||
- ( prev.tagName() == tagSeparator && !prev.attribute( \
attrWeakSeparator ).isNull() ) ||
- ( prev.tagName() == tagText ) )
- {
+ if (prev.isNull() ||
+ (equals(prev.tagName(), tagSeparator) && \
!prev.attribute(attrWeakSeparator).isNull() ) || + \
(equals(prev.tagName(), tagText))) {
// the previous element was a weak separator or didn't exist
base.removeChild( e );
continue;
@@ -346,8 +346,7 @@
// the MergeLocal tag lets us specify where non-standard elements
// of the local tree shall be merged in. After inserting the
// elements we delete this element
- else if ( tag == tagMergeLocal )
- {
+ else if (equals(tag, tagMergeLocal)) {
QDomNode it = additive.firstChild();
while ( !it.isNull() )
{
@@ -356,7 +355,7 @@
if (newChild.isNull() )
continue;
- if ( newChild.tagName() == tagText )
+ if (equals(newChild.tagName(), tagText))
continue;
if ( newChild.attribute( attrAlreadyVisited ) == attrOne )
@@ -372,7 +371,7 @@
// the global file. if it does, then we skip it as it will
// be merged in, later
QDomElement matchingElement = findMatchingElement( newChild, \
base );
- if ( matchingElement.isNull() || newChild.tagName() == \
tagSeparator ) + if (matchingElement.isNull() || \
equals(newChild.tagName(), tagSeparator)) base.insertBefore( newChild, e );
}
}
@@ -381,18 +380,19 @@
continue;
}
+ else if (equals(tag, tagText)) {
+ continue;
+ }
+ else if (equals(tag, tagMerge)) {
+ continue;
+ }
+
// in this last case we check for a separator tag and, if not, we
// can be sure that it is a container --> proceed with child nodes
// recursively and delete the just proceeded container item in
// case it is empty (if the recursive call returns true)
- else if ( tag != tagMerge )
- {
- // handle the text tag
- if ( tag == tagText )
- continue;
-
+ else {
QDomElement matchingElement = findMatchingElement( e, additive );
-
if ( !matchingElement.isNull() )
{
matchingElement.setAttribute( attrAlreadyVisited, (uint)1 );
@@ -452,8 +452,8 @@
// do one quick check to make sure that the last element was not
// a weak separator
QDomElement last = base.lastChild().toElement();
- if ( (last.tagName() == tagSeparator) && (!last.attribute( attrWeakSeparator \
).isNull()) )
- {
+ if (equals(last.tagName(), tagSeparator) &&
+ (!last.attribute(attrWeakSeparator).isNull())) {
base.removeChild( last );
}
}
@@ -473,8 +473,7 @@
const QString tag = e.tagName();
- if ( tag == tagAction )
- {
+ if (equals(tag, tagAction)) {
// if base contains an implemented action, then we must not get
// deleted (note that the actionCollection contains both,
// "global" and "local" actions
@@ -483,8 +482,7 @@
break;
}
}
- else if ( tag == tagSeparator )
- {
+ else if (equals(tag, tagSeparator)) {
// if we have a separator which has *not* the weak attribute
// set, then it must be owned by the "local" tree in which case
// we must not get deleted either
@@ -496,14 +494,12 @@
}
}
- else if ( tag == tagMerge )
- {
+ else if (equals(tag, tagMerge)) {
continue;
}
// a text tag is NOT enough to spare this container
- else if ( tag == tagText )
- {
+ else if (equals(tag, tagText)) {
continue;
}
@@ -535,16 +531,16 @@
if (e.isNull())
continue;
+ const QString tag = e.tagName();
// skip all action and merge tags as we will never use them
- if ( ( e.tagName() == tagAction ) || ( e.tagName() == tagMergeLocal ) )
- {
+ if (equals(tag, tagAction)
+ || equals(tag, tagMergeLocal)) {
continue;
}
// now see if our tags are equivalent
- if ( ( e.tagName() == base.tagName() ) &&
- ( e.attribute( attrName ) == base.attribute( attrName ) ) )
- {
+ if (equals(tag, base.tagName()) &&
+ e.attribute(attrName) == base.attribute(attrName)) {
return e;
}
}
--- trunk/KDE/kdelibs/kdeui/xmlgui/kxmlguiversionhandler.cpp #947616:947617
@@ -38,7 +38,7 @@
QList<QDomElement> toolbars;
QDomElement parent = doc.documentElement();
for (QDomElement e = parent.firstChildElement(); !e.isNull(); e = \
e.nextSiblingElement()) {
- if (e.tagName() == "ToolBar") {
+ if (e.tagName().compare(QLatin1String("ToolBar"), Qt::CaseInsensitive) == 0) \
{ toolbars.append(e);
}
}
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic