Bug 1511138 - Fix getComputedStyle usage of SelectChild. r=jaws,mconley
☠☠ backed out by 5fcc6bd202c4 ☠ ☠
authorEmilio Cobos Álvarez <emilio@crisal.io>
Thu, 06 Dec 2018 17:22:45 -0500
changeset 449658 daee82295b3c81d57991a7db1b6851dc7bc8d963
parent 449657 d23c9c3e1566f64b0257d97597daaae3c3ed0897
child 449659 5fcc6bd202c45914bdbbfcf9a8e3f3dde23c2870
push id35179
push useraciure@mozilla.com
push dateSun, 09 Dec 2018 21:43:27 +0000
treeherdermozilla-central@53fd96ca5aa4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjaws, mconley
bugs1511138
milestone65.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 1511138 - Fix getComputedStyle usage of SelectChild. r=jaws,mconley I missed the failure in browser_selectpopup_colors.js since it doesn't run on Linux. Fix the getComputedStyle usage in that code by using getDefaultComputedStyle, which is what it really wants. Also, do a bit of cleanup while at it: uaBackgroundColor was unused, and uaColor was wrong (we don't override the ua color of the <option> element, it just inherits, so it's the same as the <select> color, and that's what we were comparing it against anyway). Differential Revision: https://phabricator.services.mozilla.com/D13956
browser/base/content/test/forms/browser_selectpopup_colors.js
toolkit/actors/SelectChild.jsm
toolkit/content/widgets/browser.xml
toolkit/modules/SelectParentHelper.jsm
--- a/browser/base/content/test/forms/browser_selectpopup_colors.js
+++ b/browser/base/content/test/forms/browser_selectpopup_colors.js
@@ -180,18 +180,22 @@ const SELECT_INHERITED_COLORS_ON_OPTIONS
      <option class="textShadow">{"color": "rgb(0, 0, 255)", "textShadow": "rgb(0, 0, 0) 1px 1px 2px", "backgroundColor": "rgba(0, 0, 0, 0)"}</option>
      <option selected="true">{"end": "true"}</option>
    </select></body></html>
 `;
 
 function getSystemColor(color) {
   // Need to convert system color to RGB color.
   let textarea = document.createElementNS("http://www.w3.org/1999/xhtml", "textarea");
+  textarea.style.display = "none";
   textarea.style.color = color;
-  return getComputedStyle(textarea).color;
+  document.documentElement.appendChild(textarea);
+  let computed = getComputedStyle(textarea).color;
+  textarea.remove();
+  return computed;
 }
 
 function testOptionColors(index, item, menulist) {
   // The label contains a JSON string of the expected colors for
   // `color` and `background-color`.
   let expected = JSON.parse(item.label);
 
   for (let color of Object.keys(expected)) {
@@ -206,17 +210,17 @@ function testOptionColors(index, item, m
   EventUtils.synthesizeKey("KEY_ArrowDown");
 
   if (expected.end) {
     return;
   }
 
   if (expected.unstyled) {
     ok(!item.hasAttribute("customoptionstyling"),
-      `Item ${index} should not have any custom option styling`);
+      `Item ${index} should not have any custom option styling: ${item.outerHTML}`);
   } else {
     is(getComputedStyle(item).color, expected.color,
        "Item " + (index) + " has correct foreground color");
     is(getComputedStyle(item).backgroundColor, expected.backgroundColor,
        "Item " + (index) + " has correct background color");
     if (expected.textShadow) {
       is(getComputedStyle(item).textShadow, expected.textShadow,
          "Item " + (index) + " has correct text-shadow color");
--- a/toolkit/actors/SelectChild.jsm
+++ b/toolkit/actors/SelectChild.jsm
@@ -34,20 +34,16 @@ var gOpen = false;
 var SelectContentHelper = function(aElement, aOptions, aGlobal) {
   this.element = aElement;
   this.initialSelection = aElement[aElement.selectedIndex] || null;
   this.global = aGlobal;
   this.closedWithClickOn = false;
   this.isOpenedViaTouch = aOptions.isOpenedViaTouch;
   this._selectBackgroundColor = null;
   this._selectColor = null;
-  this._uaBackgroundColor = null;
-  this._uaColor = null;
-  this._uaSelectBackgroundColor = null;
-  this._uaSelectColor = null;
   this._closeAfterBlur = true;
   this._pseudoStylesSetup = false;
   this._lockedDescendants = null;
   this.init();
   this.showDropDown();
   this._updateTimer = new DeferredTask(this._update.bind(this), 0);
 };
 
@@ -104,29 +100,29 @@ this.SelectContentHelper.prototype = {
   showDropDown() {
     this.element.openInParentProcess = true;
     this._setupPseudoClassStyles();
     let rect = this._getBoundingContentRect();
     let computedStyles = getComputedStyles(this.element);
     this._selectBackgroundColor = computedStyles.backgroundColor;
     this._selectColor = computedStyles.color;
     this._selectTextShadow = computedStyles.textShadow;
+    let options = this._buildOptionList();
+    let defaultStyles = this.element.ownerGlobal.getDefaultComputedStyle(this.element);
     this.global.sendAsyncMessage("Forms:ShowDropDown", {
       direction: computedStyles.direction,
       isOpenedViaTouch: this.isOpenedViaTouch,
-      options: this._buildOptionList(),
+      options: options,
       rect,
       selectedIndex: this.element.selectedIndex,
       selectBackgroundColor: this._selectBackgroundColor,
       selectColor: this._selectColor,
       selectTextShadow: this._selectTextShadow,
-      uaBackgroundColor: this.uaBackgroundColor,
-      uaColor: this.uaColor,
-      uaSelectBackgroundColor: this.uaSelectBackgroundColor,
-      uaSelectColor: this.uaSelectColor,
+      uaSelectBackgroundColor: defaultStyles.backgroundColor,
+      uaSelectColor: defaultStyles.color,
     });
     this._clearPseudoClassStyles();
     gOpen = true;
   },
 
   _setupPseudoClassStyles() {
     if (this._pseudoStylesSetup) {
       throw new Error("pseudo styles must not be set up yet");
@@ -177,76 +173,30 @@ this.SelectContentHelper.prototype = {
     // Technically we might not need to set this pseudo-class
     // during _update() since the element should organically
     // have :focus, though it is here for belt-and-suspenders.
     this._setupPseudoClassStyles();
     let computedStyles = getComputedStyles(this.element);
     this._selectBackgroundColor = computedStyles.backgroundColor;
     this._selectColor = computedStyles.color;
     this._selectTextShadow = computedStyles.textShadow;
+
+    let defaultStyles = this.element.ownerGlobal.getDefaultComputedStyle(this.element);
     this.global.sendAsyncMessage("Forms:UpdateDropDown", {
       options: this._buildOptionList(),
       selectedIndex: this.element.selectedIndex,
       selectBackgroundColor: this._selectBackgroundColor,
       selectColor: this._selectColor,
       selectTextShadow: this._selectTextShadow,
-      uaBackgroundColor: this.uaBackgroundColor,
-      uaColor: this.uaColor,
-      uaSelectBackgroundColor: this.uaSelectBackgroundColor,
-      uaSelectColor: this.uaSelectColor,
+      uaSelectBackgroundColor: defaultStyles.backgroundColor,
+      uaSelectColor: defaultStyles.color,
     });
     this._clearPseudoClassStyles();
   },
 
-  // Determine user agent background-color and color.
-  // This is used to skip applying the custom color if it matches
-  // the user agent values.
-  _calculateUAColors() {
-    let dummyOption = this.element.ownerDocument.createElementNS("http://www.w3.org/1999/xhtml", "option");
-    dummyOption.style.setProperty("color", "-moz-comboboxtext", "important");
-    dummyOption.style.setProperty("background-color", "-moz-combobox", "important");
-    let optionCS = this.element.ownerGlobal.getComputedStyle(dummyOption);
-    this._uaBackgroundColor = optionCS.backgroundColor;
-    this._uaColor = optionCS.color;
-    let dummySelect = this.element.ownerDocument.createElementNS("http://www.w3.org/1999/xhtml", "select");
-    dummySelect.style.setProperty("color", "-moz-fieldtext", "important");
-    dummySelect.style.setProperty("background-color", "-moz-field", "important");
-    let selectCS = this.element.ownerGlobal.getComputedStyle(dummySelect);
-    this._uaSelectBackgroundColor = selectCS.backgroundColor;
-    this._uaSelectColor = selectCS.color;
-  },
-
-  get uaBackgroundColor() {
-    if (!this._uaBackgroundColor) {
-      this._calculateUAColors();
-    }
-    return this._uaBackgroundColor;
-  },
-
-  get uaColor() {
-    if (!this._uaColor) {
-      this._calculateUAColors();
-    }
-    return this._uaColor;
-  },
-
-  get uaSelectBackgroundColor() {
-    if (!this._selectBackgroundColor) {
-      this._calculateUAColors();
-    }
-    return this._uaSelectBackgroundColor;
-  },
-
-  get uaSelectColor() {
-    if (!this._selectBackgroundColor) {
-      this._calculateUAColors();
-    }
-    return this._uaSelectColor;
-  },
-
   dispatchMouseEvent(win, target, eventName) {
     let mouseEvent = new win.MouseEvent(eventName, {
       view: win,
       bubbles: true,
       cancelable: true,
     });
     target.dispatchEvent(mouseEvent);
   },
--- a/toolkit/content/widgets/browser.xml
+++ b/toolkit/content/widgets/browser.xml
@@ -1257,17 +1257,16 @@
               if (!this._selectParentHelper) {
                 this._selectParentHelper =
                   ChromeUtils.import("resource://gre/modules/SelectParentHelper.jsm", {}).SelectParentHelper;
               }
 
               let menulist = document.getElementById(this.getAttribute("selectmenulist"));
               menulist.menupopup.style.direction = data.direction;
               this._selectParentHelper.populate(menulist, data.options, data.selectedIndex, this._fullZoom,
-                                                data.uaBackgroundColor, data.uaColor,
                                                 data.uaSelectBackgroundColor, data.uaSelectColor,
                                                 data.selectBackgroundColor, data.selectColor, data.selectTextShadow);
               this._selectParentHelper.open(this, menulist, data.rect, data.isOpenedViaTouch);
               break;
             }
 
             case "Forms:HideDropDown": {
               if (this._selectParentHelper) {
@@ -1312,17 +1311,17 @@
               }
 
               let menulist = document.getElementById(this.getAttribute("selectmenulist"));
               menulist.menupopup.style.direction = data.direction;
 
               let zoom = Services.prefs.getBoolPref("browser.zoom.full") ||
                          this.isSyntheticDocument ? this._fullZoom : this._textZoom;
               this._selectParentHelper.populate(menulist, data.options, data.selectedIndex,
-                                                zoom, data.uaBackgroundColor, data.uaColor,
+                                                zoom,
                                                 data.uaSelectBackgroundColor, data.uaSelectColor,
                                                 data.selectBackgroundColor, data.selectColor, data.selectTextShadow);
               this._selectParentHelper.open(this, menulist, data.rect, data.isOpenedViaTouch);
               break;
             }
 
             case "FullZoomChange": {
               this._fullZoom = data.value;
--- a/toolkit/modules/SelectParentHelper.jsm
+++ b/toolkit/modules/SelectParentHelper.jsm
@@ -43,25 +43,23 @@ var SelectParentHelper = {
    * The `customoptionstyling` attribute controls the application of
    * `-moz-appearance` on the elements and is disabled if the element is
    * defining its own background-color.
    *
    * @param {Element}        menulist
    * @param {Array<Element>} items
    * @param {Number}         selectedIndex
    * @param {Number}         zoom
-   * @param {String}         uaBackgroundColor
-   * @param {String}         uaColor
    * @param {String}         uaSelectBackgroundColor
    * @param {String}         uaSelectColor
    * @param {String}         selectBackgroundColor
    * @param {String}         selectColor
    * @param {String}         selectTextShadow
    */
-  populate(menulist, items, selectedIndex, zoom, uaBackgroundColor, uaColor,
+  populate(menulist, items, selectedIndex, zoom,
            uaSelectBackgroundColor, uaSelectColor, selectBackgroundColor,
            selectColor, selectTextShadow) {
     // Clear the current contents of the popup
     menulist.menupopup.textContent = "";
     let stylesheet = menulist.querySelector("#ContentSelectDropdownStylesheet");
     if (stylesheet) {
       stylesheet.remove();
     }
@@ -94,17 +92,17 @@ var SelectParentHelper = {
     }
 
     if (customStylingEnabled &&
         selectColor != uaSelectColor &&
         selectColor != usedSelectBackgroundColor) {
       ruleBody += `color: ${selectColor};`;
       usedSelectColor = selectColor;
     } else {
-      usedSelectColor = uaColor;
+      usedSelectColor = uaSelectColor;
     }
 
     if (customStylingEnabled &&
         selectTextShadow != "none") {
       ruleBody += `text-shadow: ${selectTextShadow};`;
       sheet.insertRule(`#ContentSelectDropdown > menupopup > [_moz-menuactive="true"] {
         text-shadow: none;
       }`, 0);
@@ -247,26 +245,23 @@ var SelectParentHelper = {
         return;
       }
 
       let scrollBox = currentMenulist.menupopup.scrollBox;
       let scrollTop = scrollBox.scrollTop;
 
       let options = msg.data.options;
       let selectedIndex = msg.data.selectedIndex;
-      let uaBackgroundColor = msg.data.uaBackgroundColor;
-      let uaColor = msg.data.uaColor;
       let uaSelectBackgroundColor = msg.data.uaSelectBackgroundColor;
       let uaSelectColor = msg.data.uaSelectColor;
       let selectBackgroundColor = msg.data.selectBackgroundColor;
       let selectColor = msg.data.selectColor;
       let selectTextShadow = msg.data.selectTextShadow;
       this.populate(currentMenulist, options, selectedIndex,
-                    currentZoom, uaBackgroundColor, uaColor,
-                    uaSelectBackgroundColor, uaSelectColor,
+                    currentZoom, uaSelectBackgroundColor, uaSelectColor,
                     selectBackgroundColor, selectColor, selectTextShadow);
 
       // Restore scroll position to what it was prior to the update.
       scrollBox.scrollTop = scrollTop;
     } else if (msg.name == "Forms:BlurDropDown-Ping") {
       currentBrowser.messageManager.sendAsyncMessage("Forms:BlurDropDown-Pong", {});
     }
   },