Bug 1496137 - Handle asynchronous XBL construction of <radio> elements beneath <radiogroup>;r=jaws
authorBrian Grinstead <bgrinstead@mozilla.com>
Thu, 04 Oct 2018 20:20:10 +0000
changeset 495446 d8313ee5547ec5eb296415d8ec3e03abbc0b21d0
parent 495394 556b2f4cd653aa7642956ba66e60839b4ea9f474
child 495447 c3950445b8d1941c941f6e5f5de4f294c95f8da8
push id9984
push userffxbld-merge
push dateMon, 15 Oct 2018 21:07:35 +0000
treeherdermozilla-beta@183d27ea8570 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjaws
bugs1496137
milestone64.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1496137 - Handle asynchronous XBL construction of <radio> elements beneath <radiogroup>;r=jaws Previously, the <radio> constructor just nulled out the _radioChildren of the <radiogroup>. This leads to some issues that existed before the Custom Element migration to <radiogroup>, in which state wouldn't get synchronized between an already-appended radiogroup and a newly add radio (i.e. the [disabled] attribute on the radiogroup wouldn't copy down to the new radio, and the [value] attribute wouldn't get moved up onto the radiogroup if the new radio is [selected]). In addition to that, the Custom Element migration introduced a worse bug, in which the XBL constructors on radio elements sometime haven't run when the radio is connected. This means the radiogroup doesn't recognize any children, and the selectedItem / value is wrong. This patch makes it so that the radio will notify the radiogroup when it is constructed, and if necessary, the radiogroup can make sure all the state is consistent. Differential Revision: https://phabricator.services.mozilla.com/D7760
toolkit/content/tests/chrome/test_radio.xul
toolkit/content/widgets/radio.js
toolkit/content/widgets/radio.xml
--- a/toolkit/content/tests/chrome/test_radio.xul
+++ b/toolkit/content/tests/chrome/test_radio.xul
@@ -19,48 +19,65 @@
   <radio label="Three" value="three"/>
 </radiogroup>
 <radiogroup id="radiogroup-initwithselected" value="two">
   <radio id="one" label="One" value="one" accesskey="o"/>
   <radio id="two" label="Two" value="two" accesskey="t"/>
   <radio label="Three" value="three" selected="true"/>
 </radiogroup>
 
+<radiogroup id="radio-creation" hidden="true" />
+
   <!-- test results are displayed in the html:body -->
   <body xmlns="http://www.w3.org/1999/xhtml" style="height: 300px; overflow: auto;"/>
 
   <!-- test code goes here -->
   <script type="application/javascript"><![CDATA[
 
 SimpleTest.waitForExplicitFinish();
 
-function test_radio()
+async function test_radio()
 {
   var element = document.getElementById("radiogroup");
   test_nsIDOMXULSelectControlElement(element, "radio", null);
   test_nsIDOMXULSelectControlElement_UI(element, null);
 
   window.blur();
 
   var accessKeyDetails = (navigator.platform.includes("Mac")) ?
                          { altKey : true, ctrlKey : true } :
                          { altKey : true, shiftKey: true };
-  synthesizeKey("t", accessKeyDetails);  
+  synthesizeKey("t", accessKeyDetails);
 
   var radiogroup = $("radiogroup-initwithselected");
   is(document.activeElement, radiogroup, "accesskey focuses radiogroup");
   is(radiogroup.selectedItem, $("two"), "accesskey selects radio");
 
   $("radiogroup-initwithvalue").focus();
 
   $("one").disabled = true;
-  synthesizeKey("o", accessKeyDetails);  
+  synthesizeKey("o", accessKeyDetails);
 
   is(document.activeElement, $("radiogroup-initwithvalue"), "accesskey on disabled radio doesn't focus");
   is(radiogroup.selectedItem, $("two"), "accesskey on disabled radio doesn't change selection");
 
+  info("Testing appending child");
+  var dynamicRadiogroup = document.querySelector("#radio-creation");
+  var radio = document.createXULElement("radio");
+  radio.setAttribute("selected", "true");
+  radio.setAttribute("label", "one");
+  radio.setAttribute("value", "one");
+  dynamicRadiogroup.appendChild(radio);
+  dynamicRadiogroup.appendChild(document.createXULElement("radio"));
+
+  dynamicRadiogroup.hidden = false;
+  info("Waiting for condition");
+  await SimpleTest.promiseWaitForCondition(() => dynamicRadiogroup.value == "one",
+    "Value gets set once child is constructed");
+  is(dynamicRadiogroup._radioChildren.length, 2, "Correct number of children");
+
   SimpleTest.finish();
 }
 
 ]]>
 </script>
 
 </window>
--- a/toolkit/content/widgets/radio.js
+++ b/toolkit/content/widgets/radio.js
@@ -107,16 +107,23 @@ class MozRadiogroup extends MozBaseContr
         return;
       }
       this.removeAttribute("focused");
       this.focusedItem = null;
     });
   }
 
   connectedCallback() {
+    this.init();
+    if (!this.value) {
+      this.selectedIndex = 0;
+    }
+  }
+
+  init() {
     this._radioChildren = null;
 
     if (this.getAttribute("disabled") == "true")
       this.disabled = true;
 
     var children = this._getRadioChildren();
     var length = children.length;
     for (var i = 0; i < length; i++) {
@@ -124,18 +131,33 @@ class MozRadiogroup extends MozBaseContr
         this.selectedIndex = i;
         return;
       }
     }
 
     var value = this.value;
     if (value)
       this.value = value;
-    else
-      this.selectedIndex = 0;
+  }
+
+  /**
+   * Called when a new <radio> gets added and XBL construction happens on
+   * it. Sometimes the XBL construction happens after the <radiogroup> has
+   * already been added to the DOM. This can happen due to asynchronous XBL
+   * construction (see Bug 1496137), or just due to normal DOM appending after
+   * the <radiogroup> is created. When this happens, reinitialize the UI if
+   * necessary to make sure the state is consistent.
+   *
+   * @param {DOMNode} child
+   *                  The <radio> element that got added
+   */
+  radioChildConstructed(child) {
+    if (!this._radioChildren || !this._radioChildren.includes(child)) {
+      this.init();
+    }
   }
 
   set value(val) {
     this.setAttribute("value", val);
     var children = this._getRadioChildren();
     for (var i = 0; i < children.length; i++) {
       if (String(children[i].value) == String(val)) {
         this.selectedItem = children[i];
@@ -340,17 +362,16 @@ class MozRadiogroup extends MozBaseContr
     return (index >= 0 && index < children.length) ? children[index] : null;
   }
 
   appendItem(label, value) {
     var radio = document.createXULElement("radio");
     radio.setAttribute("label", label);
     radio.setAttribute("value", value);
     this.appendChild(radio);
-    this._radioChildren = null;
     return radio;
   }
 }
 
 MozXULElement.implementCustomInterface(MozRadiogroup, [Ci.nsIDOMXULSelectControlElement]);
 
 customElements.define("radiogroup", MozRadiogroup);
 
--- a/toolkit/content/widgets/radio.xml
+++ b/toolkit/content/widgets/radio.xml
@@ -20,17 +20,17 @@
     </content>
 
     <implementation implements="nsIDOMXULSelectControlItemElement">
       <constructor>
         <![CDATA[
           // Just clear out the parent's cached list of radio children
           var control = this.control;
           if (control)
-            control._radioChildren = null;
+            control.radioChildConstructed(this);
         ]]>
       </constructor>
       <destructor>
         <![CDATA[
           if (!this.control)
             return;
 
           var radioList = this.control._radioChildren;