Backed out changeset 5c4303496009 (bug 1386015) for browser_selectpopup_colors.js failures.
authorRyan VanderMeulen <ryanvm@gmail.com>
Wed, 13 Sep 2017 20:05:47 -0400
changeset 421721 776744d895c98d89764bca8018509a77a2127cab
parent 421720 fafd69ca273333772746a9c54ec7975a6ab1bc42
child 421722 1aadca44c4b72c06d18fc046b85444fc411d4980
push id7756
push userryanvm@gmail.com
push dateThu, 14 Sep 2017 00:25:28 +0000
treeherdermozilla-beta@faf98f63da30 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1386015
milestone56.0
backs out5c43034960096da09592861b4ad7d2ddb344d8e1
Backed out changeset 5c4303496009 (bug 1386015) for browser_selectpopup_colors.js failures.
browser/base/content/test/forms/browser_selectpopup_colors.js
toolkit/modules/SelectParentHelper.jsm
--- a/browser/base/content/test/forms/browser_selectpopup_colors.js
+++ b/browser/base/content/test/forms/browser_selectpopup_colors.js
@@ -164,29 +164,16 @@ let SELECT_LONG_WITH_TRANSITION =
 for (let i = 0; i < 75; i++) {
   SELECT_LONG_WITH_TRANSITION +=
     '  <option>{"color": "rgb(128, 0, 128)", "backgroundColor": "rgba(0, 0, 0, 0)"}</option>';
 }
 SELECT_LONG_WITH_TRANSITION +=
     '  <option selected="true">{"end": "true"}</option>' +
   "</select></body></html>";
 
-const SELECT_INHERITED_COLORS_ON_OPTIONS_DONT_GET_UNIQUE_RULES_IF_RULE_SET_ON_SELECT = `
-   <html><head><style>
-     select { color: blue; text-shadow: 1px 1px 2px blue; }
-     .redColor { color: red; }
-     .textShadow { text-shadow: 1px 1px 2px black; }
-   </style></head><body><select id='one'>
-     <option>{"color": "rgb(0, 0, 255)", "backgroundColor": "rgba(0, 0, 0, 0)"}</option>
-     <option class="redColor">{"color": "rgb(255, 0, 0)", "backgroundColor": "rgba(0, 0, 0, 0)"}</option>
-     <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.color = color;
   return getComputedStyle(textarea).color;
 }
 
 function testOptionColors(index, item, menulist) {
@@ -480,53 +467,8 @@ add_task(async function test_select_with
   let selectPopup = menulist.menupopup;
   let scrollBox = selectPopup.scrollBox;
   is(scrollBox.scrollTop, scrollBox.scrollTopMax,
     "The popup should be scrolled to the bottom of the list (where the selected item is)");
 
   await hideSelectPopup(selectPopup, "escape");
   await BrowserTestUtils.removeTab(gBrowser.selectedTab);
 });
-
-add_task(async function test_select_inherited_colors_on_options_dont_get_unique_rules_if_rule_set_on_select() {
-  let options = {
-    selectColor: "rgb(0, 0, 255)",
-    selectBgColor: "rgb(255, 255, 255)",
-    selectTextShadow: "rgb(0, 0, 255) 1px 1px 2px",
-    leaveOpen: true
-  };
-
-  await testSelectColors(SELECT_INHERITED_COLORS_ON_OPTIONS_DONT_GET_UNIQUE_RULES_IF_RULE_SET_ON_SELECT, 4, options);
-
-  let stylesheetEl = document.getElementById("ContentSelectDropdownStylesheet");
-  let sheet = stylesheetEl.sheet;
-  /* Check that there are no rulesets for the first option, but that
-     one exists for the second option and sets the color of that
-     option to "rgb(255, 0, 0)" */
-
-  function hasMatchingRuleForOption(cssRules, index, styles = {}) {
-    for (let rule of cssRules) {
-      if (rule.selectorText.includes(`:nth-child(${index})`)) {
-        if (Object.keys(styles).some(key => rule.style[key] !== styles[key])) {
-          continue;
-        }
-        return true;
-      }
-    }
-    return false;
-  }
-
-  is(hasMatchingRuleForOption(sheet.cssRules, 1), false,
-    "There should be no rules specific to option1");
-  is(hasMatchingRuleForOption(sheet.cssRules, 2, {
-    color: "rgb(255, 0, 0)"
-  }), true, "There should be a rule specific to option2 and it should have color: red");
-  is(hasMatchingRuleForOption(sheet.cssRules, 3, {
-    "text-shadow": "rgb(0, 0, 0) 1px 1px 2px"
-  }), true, "There should be a rule specific to option3 and it should have text-shadow: rgb(0, 0, 0) 1px 1px 2px");
-
-
-  let menulist = document.getElementById("ContentSelectDropdown");
-  let selectPopup = menulist.menupopup;
-
-  await hideSelectPopup(selectPopup, "escape");
-  await BrowserTestUtils.removeTab(gBrowser.selectedTab);
-});
--- a/toolkit/modules/SelectParentHelper.jsm
+++ b/toolkit/modules/SelectParentHelper.jsm
@@ -21,50 +21,22 @@ const SEARCH_MINIMUM_ELEMENTS = 40;
 // Make sure to clear these objects when the popup closes to avoid leaking.
 var currentBrowser = null;
 var currentMenulist = null;
 var selectRect = null;
 
 var currentZoom = 1;
 var closedWithEnter = false;
 var customStylingEnabled = Services.prefs.getBoolPref("dom.forms.select.customstyling");
+var usedSelectBackgroundColor;
 
 this.SelectParentHelper = {
-  /**
-   * `populate` takes the `menulist` element and a list of `items` and generates
-   * a popup list of options.
-   *
-   * If `customStylingEnabled` is set to `true`, the function will alse
-   * style the select and its popup trying to prevent the text
-   * and background to end up in the same color.
-   *
-   * All `ua*` variables represent the color values for the default colors
-   * for their respective form elements used by the user agent.
-   * The `select*` variables represent the color values defined for the
-   * particular <select> element.
-   *
-   * 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,
-           uaSelectBackgroundColor, uaSelectColor, selectBackgroundColor,
-           selectColor, selectTextShadow) {
+           uaSelectBackgroundColor, uaSelectColor, selectBackgroundColor, selectColor,
+           selectTextShadow) {
     // Clear the current contents of the popup
     menulist.menupopup.textContent = "";
     let stylesheet = menulist.querySelector("#ContentSelectDropdownStylesheet");
     if (stylesheet) {
       stylesheet.remove();
     }
 
     let doc = menulist.ownerDocument;
@@ -73,71 +45,54 @@ this.SelectParentHelper = {
       stylesheet = doc.createElementNS("http://www.w3.org/1999/xhtml", "style");
       stylesheet.setAttribute("id", "ContentSelectDropdownStylesheet");
       stylesheet.hidden = true;
       stylesheet = menulist.appendChild(stylesheet);
       sheet = stylesheet.sheet;
     }
 
     let ruleBody = "";
-    let usedSelectBackgroundColor;
-    let usedSelectColor;
-    let selectBackgroundSet = false;
 
     // Some webpages set the <select> backgroundColor to transparent,
     // but they don't intend to change the popup to transparent.
     if (customStylingEnabled &&
         selectBackgroundColor != uaSelectBackgroundColor &&
         selectBackgroundColor != "rgba(0, 0, 0, 0)") {
       ruleBody = `background-image: linear-gradient(${selectBackgroundColor}, ${selectBackgroundColor});`;
       usedSelectBackgroundColor = selectBackgroundColor;
-      selectBackgroundSet = true;
     } else {
       usedSelectBackgroundColor = uaSelectBackgroundColor;
     }
 
     if (customStylingEnabled &&
         selectColor != uaSelectColor &&
-        selectColor != usedSelectBackgroundColor) {
+        selectColor != selectBackgroundColor &&
+        (selectBackgroundColor != "rgba(0, 0, 0, 0)" ||
+         selectColor != uaSelectBackgroundColor)) {
       ruleBody += `color: ${selectColor};`;
-      usedSelectColor = selectColor;
-    } else {
-      usedSelectColor = uaColor;
     }
 
     if (customStylingEnabled &&
         selectTextShadow != "none") {
       ruleBody += `text-shadow: ${selectTextShadow};`;
-      sheet.insertRule(`#ContentSelectDropdown > menupopup > [_moz-menuactive="true"] {
-        text-shadow: none;
-      }`, 0);
     }
 
     if (ruleBody) {
       sheet.insertRule(`#ContentSelectDropdown > menupopup {
         ${ruleBody}
       }`, 0);
-      sheet.insertRule(`#ContentSelectDropdown > menupopup > :not([_moz-menuactive="true"]) {
-         color: inherit;
-      }`, 0);
-    }
-
-    // We only set the `customoptionstyling` if the background has been
-    // manually set. This prevents the overlap between moz-appearance and
-    // background-color. `color` and `text-shadow` do not interfere with it.
-    if (selectBackgroundSet) {
       menulist.menupopup.setAttribute("customoptionstyling", "true");
     } else {
       menulist.menupopup.removeAttribute("customoptionstyling");
     }
 
     currentZoom = zoom;
     currentMenulist = menulist;
     populateChildren(menulist, items, selectedIndex, zoom,
-                     usedSelectBackgroundColor, usedSelectColor, selectTextShadow, selectBackgroundSet, sheet);
+                     uaBackgroundColor, uaColor, sheet);
   },
 
   open(browser, menulist, rect, isOpenedViaTouch) {
     menulist.hidden = false;
     currentBrowser = browser;
     closedWithEnter = false;
     selectRect = rect;
     this._registerListeners(browser, menulist.menupopup);
@@ -293,46 +248,20 @@ this.SelectParentHelper = {
     browser.ownerGlobal.removeEventListener("keydown", this, true);
     browser.ownerGlobal.removeEventListener("fullscreen", this, true);
     browser.messageManager.removeMessageListener("Forms:UpdateDropDown", this);
     browser.messageManager.removeMessageListener("Forms:BlurDropDown-Ping", this);
   },
 
 };
 
-/**
- * `populateChildren` creates all <menuitem> elements for the popup menu
- * based on the list of <option> elements from the <select> element.
- *
- * It attempts to intelligently add per-item CSS rules if the single
- * item values differ from the parent menu values and attempting to avoid
- * ending up with the same color of text and background.
- *
- * @param {Element}        menulist
- * @param {Array<Element>} options
- * @param {Number}         selectedIndex
- * @param {Number}         zoom
- * @param {String}         usedSelectBackgroundColor
- * @param {String}         usedSelectColor
- * @param {String}         selectTextShadow
- * @param {String}         selectBackgroundSet
- * @param {CSSStyleSheet}  sheet
- * @param {Element}        parentElement
- * @param {Boolean}        isGroupDisabled
- * @param {Number}         adjustedTextSize
- * @param {Boolean}        addSearch
- * @param {Number}         nthChildIndex
- * @returns {Number}
- */
 function populateChildren(menulist, options, selectedIndex, zoom,
-                          usedSelectBackgroundColor, usedSelectColor,
-                          selectTextShadow, selectBackgroundSet, sheet,
+                          uaBackgroundColor, uaColor, sheet,
                           parentElement = null, isGroupDisabled = false,
-                          adjustedTextSize = -1, addSearch = true,
-                          nthChildIndex = 1) {
+                          adjustedTextSize = -1, addSearch = true, nthChildIndex = 1) {
   let element = menulist.menupopup;
   let win = element.ownerGlobal;
 
   // -1 just means we haven't calculated it yet. When we recurse through this function
   // we will pass in adjustedTextSize to save on recalculations.
   if (adjustedTextSize == -1) {
     // Grab the computed text size and multiply it by the remote browser's fullZoom to ensure
     // the popup's text size is matched with the content's. We can't just apply a CSS transform
@@ -350,60 +279,48 @@ function populateChildren(menulist, opti
     item.style.fontSize = adjustedTextSize;
     item.hidden = option.display == "none" || (parentElement && parentElement.hidden);
     // Keep track of which options are hidden by page content, so we can avoid showing
     // them on search input
     item.hiddenByContent = item.hidden;
     item.setAttribute("tooltiptext", option.tooltip);
 
     let ruleBody = "";
-    let usedBackgroundColor;
-    let optionBackgroundSet = false;
-
     if (customStylingEnabled &&
         option.backgroundColor &&
         option.backgroundColor != "rgba(0, 0, 0, 0)" &&
         option.backgroundColor != usedSelectBackgroundColor) {
       ruleBody = `background-color: ${option.backgroundColor};`;
-      usedBackgroundColor = option.backgroundColor;
-      optionBackgroundSet = true;
-    } else {
-      usedBackgroundColor = usedSelectBackgroundColor;
     }
 
     if (customStylingEnabled &&
         option.color &&
-        option.color != usedBackgroundColor &&
-        option.color != usedSelectColor) {
+        option.color != uaColor) {
       ruleBody += `color: ${option.color};`;
     }
 
     if (customStylingEnabled &&
-        option.textShadow &&
-        option.textShadow != selectTextShadow) {
+        option.textShadow) {
       ruleBody += `text-shadow: ${option.textShadow};`;
     }
 
     if (ruleBody) {
       sheet.insertRule(`#ContentSelectDropdown > menupopup > :nth-child(${nthChildIndex}):not([_moz-menuactive="true"]) {
         ${ruleBody}
       }`, 0);
 
-      if (option.textShadow && option.textShadow != selectTextShadow) {
+      if (option.textShadow) {
         // Need to explicitly disable the possibly inherited
         // text-shadow rule when _moz-menuactive=true since
         // _moz-menuactive=true disables custom option styling.
         sheet.insertRule(`#ContentSelectDropdown > menupopup > :nth-child(${nthChildIndex})[_moz-menuactive="true"] {
           text-shadow: none;
         }`, 0);
       }
-    }
 
-    if (customStylingEnabled &&
-        (optionBackgroundSet || selectBackgroundSet)) {
       item.setAttribute("customoptionstyling", "true");
     } else {
       item.removeAttribute("customoptionstyling");
     }
 
     element.appendChild(item);
     nthChildIndex++;
 
@@ -411,18 +328,17 @@ function populateChildren(menulist, opti
     let isDisabled = isGroupDisabled || option.disabled;
     if (isDisabled) {
       item.setAttribute("disabled", "true");
     }
 
     if (isOptGroup) {
       nthChildIndex =
         populateChildren(menulist, option.children, selectedIndex, zoom,
-                         usedSelectBackgroundColor, usedSelectColor,
-                         selectTextShadow, selectBackgroundSet, sheet,
+                         uaBackgroundColor, uaColor, sheet,
                          item, isDisabled, adjustedTextSize, false, nthChildIndex);
     } else {
       if (option.index == selectedIndex) {
         // We expect the parent element of the popup to be a <xul:menulist> that
         // has the popuponly attribute set to "true". This is necessary in order
         // for a <xul:menupopup> to act like a proper <html:select> dropdown, as
         // the <xul:menulist> does things like remember state and set the
         // _moz-menuactive attribute on the selected <xul:menuitem>.