Bug 1386015 - Do not generate styling for each element with inherited color. r?jaws draft
authorZibi Braniecki <zbraniecki@mozilla.com>
Fri, 11 Aug 2017 17:38:14 -0700
changeset 646692 2cc33199abb19d5ee591fd91c687f8af4775b438
parent 646498 564e82f0f289af976da01c2d50507017bbc152b5
child 726335 ea11c1dba6a6551263e4c835bcd7b43ccec52814
push id74220
push userbmo:gandalf@aviary.pl
push dateTue, 15 Aug 2017 18:48:42 +0000
reviewersjaws
bugs1386015
milestone57.0a1
Bug 1386015 - Do not generate styling for each element with inherited color. r?jaws MozReview-Commit-ID: 7myXq8aDAWr
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,16 +164,29 @@ 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) {
@@ -467,8 +480,53 @@ 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,17 +21,16 @@ 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(menulist, items, selectedIndex, zoom, uaBackgroundColor, uaColor,
            uaSelectBackgroundColor, uaSelectColor, selectBackgroundColor, selectColor,
            selectTextShadow) {
     // Clear the current contents of the popup
     menulist.menupopup.textContent = "";
     let stylesheet = menulist.querySelector("#ContentSelectDropdownStylesheet");
@@ -45,54 +44,65 @@ 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 selectBackgroundColorSet = 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;
+      selectBackgroundColorSet = true;
     } else {
       usedSelectBackgroundColor = uaSelectBackgroundColor;
     }
 
     if (customStylingEnabled &&
         selectColor != uaSelectColor &&
-        selectColor != selectBackgroundColor &&
-        (selectBackgroundColor != "rgba(0, 0, 0, 0)" ||
-         selectColor != uaSelectBackgroundColor)) {
+        selectColor != usedSelectBackgroundColor) {
       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);
       menulist.menupopup.setAttribute("customoptionstyling", "true");
     } else {
       menulist.menupopup.removeAttribute("customoptionstyling");
     }
 
     currentZoom = zoom;
     currentMenulist = menulist;
     populateChildren(menulist, items, selectedIndex, zoom,
-                     uaBackgroundColor, uaColor, sheet);
+                     usedSelectBackgroundColor, usedSelectColor, selectTextShadow, selectBackgroundColorSet, sheet);
   },
 
   open(browser, menulist, rect, isOpenedViaTouch) {
     menulist.hidden = false;
     currentBrowser = browser;
     closedWithEnter = false;
     selectRect = rect;
     this._registerListeners(browser, menulist.menupopup);
@@ -249,17 +259,18 @@ this.SelectParentHelper = {
     browser.ownerGlobal.removeEventListener("fullscreen", this, true);
     browser.messageManager.removeMessageListener("Forms:UpdateDropDown", this);
     browser.messageManager.removeMessageListener("Forms:BlurDropDown-Ping", this);
   },
 
 };
 
 function populateChildren(menulist, options, selectedIndex, zoom,
-                          uaBackgroundColor, uaColor, sheet,
+                          usedSelectBackgroundColor, usedSelectColor,
+                          selectTextShadow, selectBackgroundColorSet, sheet,
                           parentElement = null, isGroupDisabled = false,
                           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) {
@@ -279,48 +290,58 @@ 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;
+
     if (customStylingEnabled &&
         option.backgroundColor &&
         option.backgroundColor != "rgba(0, 0, 0, 0)" &&
         option.backgroundColor != usedSelectBackgroundColor) {
       ruleBody = `background-color: ${option.backgroundColor};`;
+      usedBackgroundColor = option.backgroundColor;
+    } else {
+      usedBackgroundColor = usedSelectBackgroundColor;
     }
 
     if (customStylingEnabled &&
         option.color &&
-        option.color != uaColor) {
+        option.color != usedBackgroundColor &&
+        option.color != usedSelectColor) {
       ruleBody += `color: ${option.color};`;
     }
 
     if (customStylingEnabled &&
-        option.textShadow) {
+        option.textShadow &&
+        option.textShadow != selectTextShadow) {
       ruleBody += `text-shadow: ${option.textShadow};`;
     }
 
     if (ruleBody) {
       sheet.insertRule(`#ContentSelectDropdown > menupopup > :nth-child(${nthChildIndex}):not([_moz-menuactive="true"]) {
         ${ruleBody}
       }`, 0);
 
-      if (option.textShadow) {
+      if (option.textShadow && option.textShadow != selectTextShadow) {
         // 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 &&
+        (ruleBody || selectBackgroundColorSet)) {
       item.setAttribute("customoptionstyling", "true");
     } else {
       item.removeAttribute("customoptionstyling");
     }
 
     element.appendChild(item);
     nthChildIndex++;
 
@@ -328,17 +349,18 @@ function populateChildren(menulist, opti
     let isDisabled = isGroupDisabled || option.disabled;
     if (isDisabled) {
       item.setAttribute("disabled", "true");
     }
 
     if (isOptGroup) {
       nthChildIndex =
         populateChildren(menulist, option.children, selectedIndex, zoom,
-                         uaBackgroundColor, uaColor, sheet,
+                         usedSelectBackgroundColor, usedSelectColor,
+                         selectTextShadow, selectBackgroundColorSet, 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>.