Bug 391002 - "broadcaster/command element failed to re-forward all attributes to the target element" [p=arenevier@fdn.fr (arno.) r=Neil sr=jst a1.9=damons]

content/xul/content/src/nsXULElement.cpp

@@ -1004,6 +1004,19 @@ nsXULElement::BeforeSetAttr(PRInt32 aNamespaceID, nsIAtom* aName,
             attrVal->ToString(oldValue);
             UnregisterAccessKey(oldValue);
         }
+    } 
+    else if (aNamespaceID == kNameSpaceID_None && (aName ==
+             nsGkAtoms::command || aName == nsGkAtoms::observes) && IsInDoc()) {
+//         XXX sXBL/XBL2 issue! Owner or current document?
+        nsAutoString oldValue;
+        GetAttr(kNameSpaceID_None, nsGkAtoms::observes, oldValue);
+        if (oldValue.IsEmpty()) {
+          GetAttr(kNameSpaceID_None, nsGkAtoms::command, oldValue);
+        }
+
+        if (!oldValue.IsEmpty()) {
+          RemoveBroadcaster(oldValue);
+        }
     }
 
     return nsGenericElement::BeforeSetAttr(aNamespaceID, aName,
@@ -1311,17 +1324,7 @@ nsXULElement::UnsetAttr(PRInt32 aNameSpaceID, nsIAtom* aName, PRBool aNotify)
         // need to remove our broadcaster goop completely.
         if (doc && (aName == nsGkAtoms::observes ||
                           aName == nsGkAtoms::command)) {
-            nsCOMPtr<nsIDOMXULDocument> xuldoc = do_QueryInterface(doc);
-            if (xuldoc) {
-                // Do a getElementById to retrieve the broadcaster
-                nsCOMPtr<nsIDOMElement> broadcaster;
-                nsCOMPtr<nsIDOMDocument> domDoc = do_QueryInterface(doc);
-                domDoc->GetElementById(oldValue, getter_AddRefs(broadcaster));
-                if (broadcaster) {
-                    xuldoc->RemoveBroadcastListenerFor(broadcaster, this,
-                                                       NS_LITERAL_STRING("*"));
-                }
-            }
+            RemoveBroadcaster(oldValue);
         }
     }
 
@@ -1362,6 +1365,21 @@ nsXULElement::UnsetAttr(PRInt32 aNameSpaceID, nsIAtom* aName, PRBool aNotify)
     return NS_OK;
 }
 
+void
+nsXULElement::RemoveBroadcaster(const nsAString & broadcasterId)
+{
+    nsCOMPtr<nsIDOMXULDocument> xuldoc = do_QueryInterface(GetOwnerDoc());
+    if (xuldoc) {
+        nsCOMPtr<nsIDOMElement> broadcaster;
+        nsCOMPtr<nsIDOMDocument> domDoc (do_QueryInterface(xuldoc));
+        domDoc->GetElementById(broadcasterId, getter_AddRefs(broadcaster));
+        if (broadcaster) {
+            xuldoc->RemoveBroadcastListenerFor(broadcaster, this,
+              NS_LITERAL_STRING("*"));
+        }
+    }
+}
+
 const nsAttrName*
 nsXULElement::GetAttrNameAt(PRUint32 aIndex) const
 {

content/xul/content/src/nsXULElement.h

@@ -717,6 +717,8 @@ protected:
 
     const nsAttrName* InternalGetExistingAttrNameFromQName(const nsAString& aStr) const;
 
+    void RemoveBroadcaster(const nsAString & broadcasterId);
+
 protected:
     // Internal accessor. This shadows the 'Slots', and returns
     // appropriate value.

content/xul/document/src/nsXULDocument.cpp

@@ -664,7 +664,8 @@ CanBroadcast(PRInt32 aNameSpaceID, nsIAtom* aAttribute)
 void
 nsXULDocument::SynchronizeBroadcastListener(nsIDOMElement   *aBroadcaster,
                                             nsIDOMElement   *aListener,
-                                            const nsAString &aAttr)
+                                            const nsAString &aAttr,
+                                            PRBool aAddingListener)
 {
     nsCOMPtr<nsIContent> broadcaster = do_QueryInterface(aBroadcaster);
     nsCOMPtr<nsIContent> listener = do_QueryInterface(aListener);
@@ -684,10 +685,14 @@ nsXULDocument::SynchronizeBroadcastListener(nsIDOMElement   *aBroadcaster,
             if (! CanBroadcast(nameSpaceID, name))
                 continue;
 
-            nsAutoString value;
-            broadcaster->GetAttr(nameSpaceID, name, value);
-            listener->SetAttr(nameSpaceID, name, attrName->GetPrefix(), value,
-                              PR_FALSE);
+            if (aAddingListener) {
+                nsAutoString value;
+                broadcaster->GetAttr(nameSpaceID, name, value);
+                listener->SetAttr(nameSpaceID, name, attrName->GetPrefix(), 
+                                  value, mInitialLayoutComplete);
+            } else {
+                listener->UnsetAttr(nameSpaceID, name, mInitialLayoutComplete);
+            }
 
 #if 0
             // XXX we don't fire the |onbroadcast| handler during
@@ -704,11 +709,13 @@ nsXULDocument::SynchronizeBroadcastListener(nsIDOMElement   *aBroadcaster,
         nsCOMPtr<nsIAtom> name = do_GetAtom(aAttr);
 
         nsAutoString value;
-        if (broadcaster->GetAttr(kNameSpaceID_None, name, value)) {
-            listener->SetAttr(kNameSpaceID_None, name, value, PR_FALSE);
+        if (broadcaster->GetAttr(kNameSpaceID_None, name, value) 
+            && aAddingListener) {
+            listener->SetAttr(kNameSpaceID_None, name, value,
+                              mInitialLayoutComplete);
         }
         else {
-            listener->UnsetAttr(kNameSpaceID_None, name, PR_FALSE);
+            listener->UnsetAttr(kNameSpaceID_None, name, mInitialLayoutComplete);
         }
 
 #if 0
@@ -804,7 +811,7 @@ nsXULDocument::AddBroadcastListenerFor(nsIDOMElement* aBroadcaster,
 
     entry->mListeners.AppendElement(bl);
 
-    SynchronizeBroadcastListener(aBroadcaster, aListener, aAttr);
+    SynchronizeBroadcastListener(aBroadcaster, aListener, aAttr, PR_TRUE);
     return NS_OK;
 }
 
@@ -837,7 +844,7 @@ nsXULDocument::RemoveBroadcastListenerFor(nsIDOMElement* aBroadcaster,
                     PL_DHashTableOperate(mBroadcasterMap, aBroadcaster,
                                          PL_DHASH_REMOVE);
 
-                SynchronizeBroadcastListener(aBroadcaster, aListener, aAttr);
+                SynchronizeBroadcastListener(aBroadcaster, aListener, aAttr, PR_FALSE);
 
                 break;
             }
@@ -975,6 +982,10 @@ nsXULDocument::AttributeChanged(nsIDocument* aDocument,
         }
     }
 
+    // checks for modifications in broadcasters
+    PRBool listener, resolved;
+    CheckBroadcasterHookup(aElement, &listener, &resolved);
+
     // See if there is anything we need to persist in the localstore.
     //
     // XXX Namespace handling broken :-(

content/xul/document/src/nsXULDocument.h

@@ -528,7 +528,8 @@ protected:
     void
     SynchronizeBroadcastListener(nsIDOMElement   *aBroadcaster,
                                  nsIDOMElement   *aListener,
-                                 const nsAString &aAttr);
+                                 const nsAString &aAttr,
+                                 PRBool aAddingListener);
 
     static
     nsresult

content/xul/document/test/Makefile.in

@@ -47,6 +47,7 @@ include $(topsrcdir)/config/rules.mk
 _TEST_FILES	= \
 		test_bug311681.xul \
 		test_bug199692.xul \
+		test_bug391002.xul \
 		test_bug403868.xul \
 		$(NULL)
 

content/xul/document/test/test_bug391002.xul

@@ -0,0 +1,48 @@
+<?xml version="1.0"?>
+<?xml-stylesheet href="chrome://global/skin" type="text/css"?>
+<?xml-stylesheet href="/tests/SimpleTest/test.css" type="text/css"?>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=391002
+-->
+<window title="Mozilla Bug 391002"
+  xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul">
+  <script type="application/javascript" src="/MochiKit/packed.js" />
+  <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"/>
+
+  <!-- test resuls are displayed in the html:body -->
+  <body xmlns="http://www.w3.org/1999/xhtml">
+  <a href="https://bugzilla.mozilla.org/show_bug.cgi?id=391002"
+     target="_blank">Mozilla Bug 391002</a>
+  </body>
+
+  <button id="btn1" command="cmd1"/>
+
+  <button id="btn2">
+    <observes id="observes" element="cmd1" attribute="label"/>
+  </button>
+
+  <commandset>
+    <command id="cmd1" label="cmd1"/>
+    <command id="cmd2" label="cmd2"/>
+  </commandset>
+
+  <!-- test code goes here -->
+  <script type="application/javascript"><![CDATA[
+
+    /** Test for Bug 391002 **/
+
+    $("btn1").setAttribute("command", "cmd2");
+    is($("btn1").getAttribute("label"), $("cmd2").getAttribute("label"))
+
+    $("btn1").removeAttribute("command");
+    is($("btn1").getAttribute("label"), "");
+
+    $("observes").setAttribute("element", "cmd2");
+    is($("btn2").getAttribute("label"), $("cmd2").getAttribute("label"))
+
+    $("observes").removeAttribute("element");
+    todo_is($("btn2").getAttribute("label"), "");
+
+  ]]></script>
+</window>
+