Bug 1530709 - Restrict styles to known selectors and single rules per message. r=emilio, a=RyanVM
authorFrederik Braun <fbraun@mozilla.com>
Fri, 13 Sep 2019 14:30:58 +0000
changeset 555129 c97e152648be55d4daef8239bafa288aeb797f0e
parent 555128 760ada8864988f50085f34b09b00e308dc9c3066
child 555130 89f6acce40bf2f2fffc3d49f21e219e9e8003123
push id2165
push userffxbld-merge
push dateMon, 14 Oct 2019 16:30:58 +0000
treeherdermozilla-release@0eae18af659f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersemilio, RyanVM
bugs1530709
milestone70.0
Bug 1530709 - Restrict styles to known selectors and single rules per message. r=emilio, a=RyanVM Differential Revision: https://phabricator.services.mozilla.com/D43945
toolkit/actors/SelectChild.jsm
toolkit/actors/SelectParent.jsm
--- a/toolkit/actors/SelectChild.jsm
+++ b/toolkit/actors/SelectChild.jsm
@@ -21,16 +21,18 @@ ChromeUtils.defineModuleGetter(
   "resource://gre/modules/DeferredTask.jsm"
 );
 
 XPCOMUtils.defineLazyGlobalGetters(this, ["InspectorUtils"]);
 
 const kStateActive = 0x00000001; // NS_EVENT_STATE_ACTIVE
 const kStateHover = 0x00000004; // NS_EVENT_STATE_HOVER
 
+// Duplicated in SelectParent.jsm
+// Please keep these lists in sync.
 const SUPPORTED_PROPERTIES = [
   "direction",
   "color",
   "background-color",
   "text-shadow",
   "font-family",
   "font-weight",
   "font-size",
--- a/toolkit/actors/SelectParent.jsm
+++ b/toolkit/actors/SelectParent.jsm
@@ -19,16 +19,29 @@ const SEARCH_MINIMUM_ELEMENTS = 40;
 
 // The properties that we should respect only when the item is not active.
 const PROPERTIES_RESET_WHEN_ACTIVE = [
   "color",
   "background-color",
   "text-shadow",
 ];
 
+// Duplicated in SelectChild.jsm
+// Please keep these lists in sync.
+const SUPPORTED_PROPERTIES = [
+  "direction",
+  "color",
+  "background-color",
+  "text-shadow",
+  "font-family",
+  "font-weight",
+  "font-size",
+  "font-style",
+];
+
 const customStylingEnabled = Services.prefs.getBoolPref(
   "dom.forms.select.customstyling"
 );
 
 var SelectParentHelper = {
   /**
    * `populate` takes the `menulist` element and a list of `items` and generates
    * a popup list of options.
@@ -89,57 +102,65 @@ var SelectParentHelper = {
     }
 
     let selectBackgroundSet = false;
 
     if (selectStyle["background-color"] == "rgba(0, 0, 0, 0)") {
       selectStyle["background-color"] = uaStyle["background-color"];
     }
 
-    // Some webpages set the <select> backgroundColor to transparent,
-    // but they don't intend to change the popup to transparent.
-    if (
-      customStylingEnabled &&
-      selectStyle["background-color"] != uaStyle["background-color"]
-    ) {
-      let color = selectStyle["background-color"];
-      selectStyle["background-image"] = `linear-gradient(${color}, ${color});`;
-      selectBackgroundSet = true;
-    }
-
     if (selectStyle.color == selectStyle["background-color"]) {
       selectStyle.color = uaStyle.color;
     }
 
     if (customStylingEnabled) {
       if (selectStyle["text-shadow"] != "none") {
         sheet.insertRule(
           `#ContentSelectDropdown > menupopup > [_moz-menuactive="true"] {
           text-shadow: none;
         }`,
           0
         );
       }
 
-      let ruleBody = "";
-      for (let property in selectStyle) {
-        if (property == "background-color" || property == "direction") {
+      let addedRule = false;
+      for (let property of SUPPORTED_PROPERTIES) {
+        if (property == "direction") {
           continue;
         } // Handled above, or before.
-        if (selectStyle[property] != uaStyle[property]) {
-          ruleBody += `${property}: ${selectStyle[property]};`;
+        if (
+          !selectStyle[property] ||
+          selectStyle[property] == uaStyle[property]
+        ) {
+          continue;
         }
+        if (!addedRule) {
+          sheet.insertRule("#ContentSelectDropdown > menupopup {}", 0);
+          addedRule = true;
+        }
+        sheet.cssRules[0].style[property] = selectStyle[property];
       }
-      if (ruleBody) {
-        sheet.insertRule(
-          `#ContentSelectDropdown > menupopup {
-          ${ruleBody}
-        }`,
-          0
-        );
+      // Some webpages set the <select> backgroundColor to transparent,
+      // but they don't intend to change the popup to transparent.
+      // So we remove the backgroundColor and turn it into an image instead.
+      if (
+        customStylingEnabled &&
+        selectStyle["background-color"] != uaStyle["background-color"]
+      ) {
+        // We intentionally use the parsed color to prevent color
+        // values like `url(..)` being injected into the
+        // `background-image` property.
+        let parsedColor = sheet.cssRules[0].style["background-color"];
+        sheet.cssRules[0].style["background-color"] = "";
+        sheet.cssRules[0].style[
+          "background-image"
+        ] = `linear-gradient(${parsedColor}, ${parsedColor})`;
+        selectBackgroundSet = true;
+      }
+      if (addedRule) {
         sheet.insertRule(
           `#ContentSelectDropdown > menupopup > :not([_moz-menuactive="true"]) {
             color: inherit;
         }`,
           0
         );
       }
     }
@@ -411,39 +432,40 @@ var SelectParentHelper = {
       let optionBackgroundSet =
         style["background-color"] != selectStyle["background-color"];
 
       if (style.color == style["background-color"]) {
         style.color = selectStyle.color;
       }
 
       if (customStylingEnabled) {
-        let ruleBody = "";
-        for (let property in style) {
+        let addedRule = false;
+        for (const property of SUPPORTED_PROPERTIES) {
           if (property == "direction" || property == "font-size") {
             continue;
           } // handled above
-          if (style[property] == selectStyle[property]) {
+          if (!style[property] || style[property] == selectStyle[property]) {
             continue;
           }
           if (PROPERTIES_RESET_WHEN_ACTIVE.includes(property)) {
-            ruleBody += `${property}: ${style[property]};`;
+            if (!addedRule) {
+              sheet.insertRule(
+                `#ContentSelectDropdown > menupopup > :nth-child(${nthChildIndex}):not([_moz-menuactive="true"]) {
+              }`,
+                0
+              );
+              addedRule = true;
+            }
+            sheet.cssRules[0].style[property] = style[property];
           } else {
             item.style.setProperty(property, style[property]);
           }
         }
 
-        if (ruleBody) {
-          sheet.insertRule(
-            `#ContentSelectDropdown > menupopup > :nth-child(${nthChildIndex}):not([_moz-menuactive="true"]) {
-            ${ruleBody}
-          }`,
-            0
-          );
-
+        if (addedRule) {
           if (
             style["text-shadow"] != "none" &&
             style["text-shadow"] != selectStyle["text-shadow"]
           ) {
             // Need to explicitly disable the possibly inherited
             // text-shadow rule when _moz-menuactive=true since
             // _moz-menuactive=true disables custom option styling.
             sheet.insertRule(