Bug 1386015 - Do not generate styling for each element with inherited color. r?jaws
This patch does a minor refactor of the code used to style popup menu for
the <select> element.
It improves the custom styling experience on MacOS, preserves the functionality
on Windows and removes the unnecessary per-item CSS rules significantly
improving the performance of opening the <select> list.
MozReview-Commit-ID: 7myXq8aDAWr
--- 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,22 +21,50 @@ 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;
@@ -45,54 +73,71 @@ 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 != 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);
+ }
+
+ // 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,
- uaBackgroundColor, uaColor, sheet);
+ usedSelectBackgroundColor, usedSelectColor, selectTextShadow, selectBackgroundSet, sheet);
},
open(browser, menulist, rect, isOpenedViaTouch) {
menulist.hidden = false;
currentBrowser = browser;
closedWithEnter = false;
selectRect = rect;
this._registerListeners(browser, menulist.menupopup);
@@ -248,20 +293,46 @@ 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,
- uaBackgroundColor, uaColor, sheet,
+ usedSelectBackgroundColor, usedSelectColor,
+ selectTextShadow, selectBackgroundSet, 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
@@ -279,48 +350,60 @@ 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 != 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 &&
+ (optionBackgroundSet || selectBackgroundSet)) {
item.setAttribute("customoptionstyling", "true");
} else {
item.removeAttribute("customoptionstyling");
}
element.appendChild(item);
nthChildIndex++;
@@ -328,17 +411,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, selectBackgroundSet, 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>.