From 93673abdb95ab46ab3d941d3f063a2fbd0f71502 Mon Sep 17 00:00:00 2001 From: Michael Ratcliffe Date: Wed, 17 Dec 2014 13:43:30 +0000 Subject: [PATCH] Bug 1111601 - Avoid using hiddenwindow in DevTools code r=pbrosset --- browser/devtools/eyedropper/eyedropper.js | 5 +- .../projecteditor/lib/stores/local.js | 10 +- .../devtools/shared/test/browser_css_color.js | 2 +- browser/devtools/styleinspector/rule-view.js | 26 +--- toolkit/devtools/css-color.js | 130 +++++++----------- toolkit/devtools/output-parser.js | 10 +- 6 files changed, 57 insertions(+), 126 deletions(-) diff --git a/browser/devtools/eyedropper/eyedropper.js b/browser/devtools/eyedropper/eyedropper.js index cb2992dbaa3b..e0a3f816bd53 100644 --- a/browser/devtools/eyedropper/eyedropper.js +++ b/browser/devtools/eyedropper/eyedropper.js @@ -6,6 +6,7 @@ const {Cc, Ci, Cu} = require("chrome"); const {rgbToHsl} = require("devtools/css-color").colorUtils; const {EventEmitter} = Cu.import("resource://gre/modules/devtools/event-emitter.js"); const promise = Cu.import("resource://gre/modules/Promise.jsm", {}).Promise; +const {setTimeout, clearTimeout} = Cu.import("resource://gre/modules/Timer.jsm", {}); Cu.import("resource://gre/modules/Services.jsm"); @@ -546,7 +547,7 @@ Eyedropper.prototype = { * Callback to be called when the color is in the clipboard. */ copyColor: function(callback) { - Services.appShell.hiddenDOMWindow.clearTimeout(this._copyTimeout); + clearTimeout(this._copyTimeout); let color = this._colorValue.value; clipboardHelper.copyString(color); @@ -554,7 +555,7 @@ Eyedropper.prototype = { this._colorValue.classList.add("highlight"); this._colorValue.value = "✓ " + l10n.GetStringFromName("colorValue.copied"); - this._copyTimeout = Services.appShell.hiddenDOMWindow.setTimeout(() => { + this._copyTimeout = setTimeout(() => { this._colorValue.classList.remove("highlight"); this._colorValue.value = color; diff --git a/browser/devtools/projecteditor/lib/stores/local.js b/browser/devtools/projecteditor/lib/stores/local.js index 0053966124af..7fdc42107bcc 100644 --- a/browser/devtools/projecteditor/lib/stores/local.js +++ b/browser/devtools/projecteditor/lib/stores/local.js @@ -14,6 +14,7 @@ const promise = require("projecteditor/helpers/promise"); const { on, forget } = require("projecteditor/helpers/event"); const { FileResource } = require("projecteditor/stores/resource"); const {Services} = Cu.import("resource://gre/modules/Services.jsm"); +const {setTimeout, clearTimeout} = Cu.import("resource://gre/modules/Timer.jsm", {}); const CHECK_LINKED_DIRECTORY_DELAY = 5000; const SHOULD_LIVE_REFRESH = true; @@ -35,7 +36,6 @@ var LocalStore = Class({ initialize: function(path) { this.initStore(); - this.window = Services.appShell.hiddenDOMWindow; this.path = OS.Path.normalize(path); this.rootPath = this.path; this.displayName = this.path; @@ -46,9 +46,8 @@ var LocalStore = Class({ }, destroy: function() { - if (this.window) { - this.window.clearTimeout(this._refreshTimeout); - } + clearTimeout(this._refreshTimeout); + if (this._refreshDeferred) { this._refreshDeferred.reject("destroy"); } @@ -58,7 +57,6 @@ var LocalStore = Class({ this._refreshTimeout = null; this._refreshDeferred = null; - this.window = null; this.worker = null; if (this.root) { @@ -132,7 +130,7 @@ var LocalStore = Class({ // XXX: Once Bug 958280 adds a watch function, will not need to forever loop here. this.refresh().then(() => { if (SHOULD_LIVE_REFRESH) { - this._refreshTimeout = this.window.setTimeout(this.refreshLoop, + this._refreshTimeout = setTimeout(this.refreshLoop, CHECK_LINKED_DIRECTORY_DELAY); } }); diff --git a/browser/devtools/shared/test/browser_css_color.js b/browser/devtools/shared/test/browser_css_color.js index e7efcc851502..ab498db791dc 100644 --- a/browser/devtools/shared/test/browser_css_color.js +++ b/browser/devtools/shared/test/browser_css_color.js @@ -17,7 +17,7 @@ function test() { waitForFocus(init, content); }, true); - content.location = "data:text/html,browser_css_color.js"; + content.location = "data:text/html;charset=utf-8,browser_css_color.js"; } function init() { diff --git a/browser/devtools/styleinspector/rule-view.js b/browser/devtools/styleinspector/rule-view.js index 0cfe6ec40b32..dad2872c5896 100644 --- a/browser/devtools/styleinspector/rule-view.js +++ b/browser/devtools/styleinspector/rule-view.js @@ -2822,32 +2822,10 @@ TextPropertyEditor.prototype = { * Validate this property. Does it make sense for this value to be assigned * to this property name? This does not apply the property value * - * @param {string} [aValue] - * The property value used for validation. - * Defaults to the current value for this.prop - * * @return {bool} true if the property value is valid, false otherwise. */ - isValid: function(aValue) { - let name = this.prop.name; - let value = typeof aValue == "undefined" ? this.prop.value : aValue; - let val = parseSingleValue(value); - - let style = this.doc.createElementNS(HTML_NS, "div").style; - let prefs = Services.prefs; - - // We toggle output of errors whilst the user is typing a property value. - let prefVal = prefs.getBoolPref("layout.css.report_errors"); - prefs.setBoolPref("layout.css.report_errors", false); - - let validValue = false; - try { - style.setProperty(name, val.value, val.priority); - validValue = style.getPropertyValue(name) !== "" || val.value === ""; - } finally { - prefs.setBoolPref("layout.css.report_errors", prefVal); - } - return validValue; + isValid: function() { + return domUtils.cssPropertyIsValid(this.prop.name, this.prop.value); } }; diff --git a/toolkit/devtools/css-color.js b/toolkit/devtools/css-color.js index 170e24568228..da5ef242ba12 100644 --- a/toolkit/devtools/css-color.js +++ b/toolkit/devtools/css-color.js @@ -10,8 +10,6 @@ const {Services} = Cu.import("resource://gre/modules/Services.jsm", {}); const COLOR_UNIT_PREF = "devtools.defaultColorUnit"; const REGEX_JUST_QUOTES = /^""$/; -const REGEX_RGB_3_TUPLE = /^rgb\(([\d.]+),\s*([\d.]+),\s*([\d.]+)\)$/i; -const REGEX_RGBA_4_TUPLE = /^rgba\(([\d.]+),\s*([\d.]+),\s*([\d.]+),\s*([\d.]+|1|0)\)$/i; const REGEX_HSL_3_TUPLE = /^\bhsl\(([\d.]+),\s*([\d.]+%),\s*([\d.]+%)\)$/i; /** @@ -106,7 +104,7 @@ CssColor.prototype = { }, get valid() { - return this._validateColor(this.authored); + return DOMUtils.isValidCSSColor(this.authored); }, /** @@ -126,11 +124,9 @@ CssColor.prototype = { }, get name() { - if (!this.valid) { - return ""; - } - if (this.specialValue) { - return this.specialValue; + let invalidOrSpecialValue = this._getInvalidOrSpecialValue(); + if (invalidOrSpecialValue !== false) { + return invalidOrSpecialValue; } try { @@ -147,11 +143,9 @@ CssColor.prototype = { }, get hex() { - if (!this.valid) { - return ""; - } - if (this.specialValue) { - return this.specialValue; + let invalidOrSpecialValue = this._getInvalidOrSpecialValue(); + if (invalidOrSpecialValue !== false) { + return invalidOrSpecialValue; } if (this.hasAlpha) { return this.rgba; @@ -167,11 +161,9 @@ CssColor.prototype = { }, get longHex() { - if (!this.valid) { - return ""; - } - if (this.specialValue) { - return this.specialValue; + let invalidOrSpecialValue = this._getInvalidOrSpecialValue(); + if (invalidOrSpecialValue !== false) { + return invalidOrSpecialValue; } if (this.hasAlpha) { return this.rgba; @@ -182,11 +174,9 @@ CssColor.prototype = { }, get rgb() { - if (!this.valid) { - return ""; - } - if (this.specialValue) { - return this.specialValue; + let invalidOrSpecialValue = this._getInvalidOrSpecialValue(); + if (invalidOrSpecialValue !== false) { + return invalidOrSpecialValue; } if (!this.hasAlpha) { if (this.authored.startsWith("rgb(")) { @@ -200,11 +190,9 @@ CssColor.prototype = { }, get rgba() { - if (!this.valid) { - return ""; - } - if (this.specialValue) { - return this.specialValue; + let invalidOrSpecialValue = this._getInvalidOrSpecialValue(); + if (invalidOrSpecialValue !== false) { + return invalidOrSpecialValue; } if (this.authored.startsWith("rgba(")) { // The color is valid and begins with rgba(. Return the authored value. @@ -218,11 +206,9 @@ CssColor.prototype = { }, get hsl() { - if (!this.valid) { - return ""; - } - if (this.specialValue) { - return this.specialValue; + let invalidOrSpecialValue = this._getInvalidOrSpecialValue(); + if (invalidOrSpecialValue !== false) { + return invalidOrSpecialValue; } if (this.authored.startsWith("hsl(")) { // The color is valid and begins with hsl(. Return the authored value. @@ -235,11 +221,9 @@ CssColor.prototype = { }, get hsla() { - if (!this.valid) { - return ""; - } - if (this.specialValue) { - return this.specialValue; + let invalidOrSpecialValue = this._getInvalidOrSpecialValue(); + if (invalidOrSpecialValue !== false) { + return invalidOrSpecialValue; } if (this.authored.startsWith("hsla(")) { // The color is valid and begins with hsla(. Return the authored value. @@ -252,6 +236,27 @@ CssColor.prototype = { return this._hslNoAlpha().replace("hsl", "hsla").replace(")", ", 1)"); }, + /** + * Check whether the current color value is in the special list e.g. + * transparent or invalid. + * + * @return {String|Boolean} + * - If the current color is a special value e.g. "transparent" then + * return the color. + * - If the color is invalid return an empty string. + * - If the color is a regular color e.g. #F06 so we return false + * to indicate that the color is neither invalid or special. + */ + _getInvalidOrSpecialValue: function() { + if (this.specialValue) { + return this.specialValue; + } + if (!this.valid) { + return ""; + } + return false; + }, + /** * Change color * @@ -298,27 +303,11 @@ CssColor.prototype = { * appropriate. */ _getRGBATuple: function() { - let win = Services.appShell.hiddenDOMWindow; - let doc = win.document; - let span = doc.createElement("span"); - span.style.color = this.authored; - let computed = win.getComputedStyle(span).color; + let tuple = DOMUtils.colorToRGBA(this.authored); - if (computed === "transparent") { - return {r: 0, g: 0, b: 0, a: 0}; - } + tuple.a = parseFloat(tuple.a.toFixed(1)); - let rgba = computed.match(REGEX_RGBA_4_TUPLE); - - if (rgba) { - let [, r, g, b, a] = rgba; - return {r: r, g: g, b: b, a: a}; - } else { - let rgb = computed.match(REGEX_RGB_3_TUPLE); - let [, r, g, b] = rgb; - - return {r: r, g: g, b: b, a: 1}; - } + return tuple; }, _hslNoAlpha: function() { @@ -342,33 +331,6 @@ CssColor.prototype = { valueOf: function() { return this.rgba; }, - - _validateColor: function(color) { - if (typeof color !== "string" || color === "") { - return false; - } - - let win = Services.appShell.hiddenDOMWindow; - let doc = win.document; - - // Create a black span in a hidden window. - let span = doc.createElement("span"); - span.style.color = "rgb(0, 0, 0)"; - - // Attempt to set the color. If the color is no longer black we know that - // color is valid. - span.style.color = color; - if (span.style.color !== "rgb(0, 0, 0)") { - return true; - } - - // If the color is black then the above check will have failed. We change - // the span to white and attempt to reapply the color. If the span is not - // white then we know that the color is valid otherwise we return invalid. - span.style.color = "rgb(255, 255, 255)"; - span.style.color = color; - return span.style.color !== "rgb(255, 255, 255)"; - }, }; /** diff --git a/toolkit/devtools/output-parser.js b/toolkit/devtools/output-parser.js index 902393ceea69..469b4e117a3c 100644 --- a/toolkit/devtools/output-parser.js +++ b/toolkit/devtools/output-parser.js @@ -337,15 +337,7 @@ OutputParser.prototype = { * CSS Property value to check */ _cssPropertySupportsValue: function(name, value) { - let win = Services.appShell.hiddenDOMWindow; - let doc = win.document; - - value = value.replace("!important", ""); - - let div = doc.createElement("div"); - div.style.setProperty(name, value); - - return !!div.style.getPropertyValue(name); + return DOMUtils.cssPropertyIsValid(name, value); }, /**