Bug 1530294 - Add option to skip cache when requesting authored text for CSS rule r=gl
authorRazvan Caliman <rcaliman@mozilla.com>
Tue, 26 Feb 2019 09:09:01 +0000
changeset 518924 ccfe228bff77703ea43e78e8107222736e389998
parent 518923 c0df44b239b51a159441cbc04bdf954ec334e385
child 518925 9b617c97285a7757e86ae94b5e974be93074a4f0
push id10862
push userffxbld-merge
push dateMon, 11 Mar 2019 13:01:11 +0000
treeherdermozilla-beta@a2e7f5c935da [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgl
bugs1530294
milestone67.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1530294 - Add option to skip cache when requesting authored text for CSS rule r=gl This patch adds a boolean option to skip the cached value of `StyeRuleActor.authoredText` and re-parse the stylesheet for its value. When changing content of a descendant rule inside an ancestor like @media or @select, the cached `authoredText` value is outdated. This yields incorrect data when requesting the complete rule authored text, hence the need for a cache-busting option. In addition to this change, there is a slight refactor to include the generated unique selector for the mock-rule for element inline styles. Differential Revision: https://phabricator.services.mozilla.com/D21033
devtools/client/inspector/changes/ChangesView.js
devtools/server/actors/styles.js
--- a/devtools/client/inspector/changes/ChangesView.js
+++ b/devtools/client/inspector/changes/ChangesView.js
@@ -5,17 +5,16 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 "use strict";
 
 const { createFactory, createElement } = require("devtools/client/shared/vendor/react");
 const { Provider } = require("devtools/client/shared/vendor/react-redux");
 
 loader.lazyRequireGetter(this, "ChangesContextMenu", "devtools/client/inspector/changes/ChangesContextMenu");
-loader.lazyRequireGetter(this, "prettifyCSS", "devtools/shared/inspector/css-logic", true);
 loader.lazyRequireGetter(this, "clipboardHelper", "devtools/shared/platform/clipboard");
 
 const ChangesApp = createFactory(require("./components/ChangesApp"));
 const { getChangesStylesheet } = require("./selectors/changes");
 
 const {
   TELEMETRY_SCALAR_CONTEXTMENU,
   TELEMETRY_SCALAR_CONTEXTMENU_COPY,
@@ -153,18 +152,17 @@ class ChangesView {
    * and copies it to the clipboard.
    *
    * @param {String} ruleId
    *        Rule id of the target CSS rule.
    */
   async copyRule(ruleId) {
     const rule = await this.inspector.pageStyle.getRule(ruleId);
     const text = await rule.getRuleText();
-    const prettyCSS = prettifyCSS(text);
-    clipboardHelper.copyString(prettyCSS);
+    clipboardHelper.copyString(text);
   }
 
   /**
    * Handler for the "Copy" option from the context menu.
    * Copies the current text selection to the clipboard.
    */
   copySelection() {
     clipboardHelper.copyString(this.window.getSelection().toString());
--- a/devtools/server/actors/styles.js
+++ b/devtools/server/actors/styles.js
@@ -18,16 +18,18 @@ const {pageStyleSpec, styleRuleSpec, ELE
 loader.lazyRequireGetter(this, "CssLogic", "devtools/server/actors/inspector/css-logic", true);
 loader.lazyRequireGetter(this, "SharedCssLogic", "devtools/shared/inspector/css-logic");
 loader.lazyRequireGetter(this, "getDefinedGeometryProperties",
   "devtools/server/actors/highlighters/geometry-editor", true);
 loader.lazyRequireGetter(this, "isCssPropertyKnown",
   "devtools/server/actors/css-properties", true);
 loader.lazyRequireGetter(this, "parseNamedDeclarations",
   "devtools/shared/css/parsing-utils", true);
+loader.lazyRequireGetter(this, "prettifyCSS",
+  "devtools/shared/inspector/css-logic", true);
 loader.lazyRequireGetter(this, "UPDATE_PRESERVING_RULES",
   "devtools/server/actors/stylesheets", true);
 loader.lazyRequireGetter(this, "UPDATE_GENERAL",
   "devtools/server/actors/stylesheets", true);
 loader.lazyRequireGetter(this, "findCssSelector",
   "devtools/shared/inspector/css-logic", true);
 loader.lazyRequireGetter(this, "CSSRuleTypeName",
   "devtools/shared/inspector/css-logic", true);
@@ -1392,23 +1394,28 @@ var StyleRuleActor = protocol.ActorClass
 
   /**
    * Return a promise that resolves to the authored form of a rule's
    * text, if available.  If the authored form is not available, the
    * returned promise simply resolves to the empty string.  If the
    * authored form is available, this also sets |this.authoredText|.
    * The authored text will include invalid and otherwise ignored
    * properties.
+   *
+   * @param {Boolean} skipCache
+   *        If a value for authoredText was previously found and cached,
+   *        ignore it and parse the stylehseet again. The authoredText
+   *        may be outdated if a descendant of this rule has changed.
    */
-  getAuthoredCssText: function() {
+  getAuthoredCssText: function(skipCache = false) {
     if (!this.canSetRuleText || !SUPPORTED_RULE_TYPES.includes(this.type)) {
       return Promise.resolve("");
     }
 
-    if (typeof this.authoredText === "string") {
+    if (typeof this.authoredText === "string" && !skipCache) {
       return Promise.resolve(this.authoredText);
     }
 
     return this.sheetActor.getText().then((longStr) => {
       const cssText = longStr.str;
       const {text} = getRuleText(cssText, this.line, this.column);
 
       // Cache the result on the rule actor to avoid parsing again next time
@@ -1419,48 +1426,57 @@ var StyleRuleActor = protocol.ActorClass
 
   /**
    * Return a promise that resolves to the complete cssText of the rule as authored.
    *
    * Unlike |getAuthoredCssText()|, which only returns the contents of the rule, this
    * method includes the CSS selectors and at-rules (@media, @supports, @keyframes, etc.)
    *
    * If the rule type is unrecongized, the promise resolves to an empty string.
-   * If the rule is an element inline style, the promise resolves to the text content of
+   * If the rule is an element inline style, the promise resolves with the generated
+   * selector that uniquely identifies the element and with the rule body consisting of
    * the element's style attribute.
    *
    * @return {String}
    */
   getRuleText: async function() {
-    if (this.type === ELEMENT_STYLE) {
-      return Promise.resolve(this.rawNode.getAttribute("style"));
-    }
-
-    if (!SUPPORTED_RULE_TYPES.includes(this.type)) {
+    // Bail out if the rule is not supported or not an element inline style.
+    if (![...SUPPORTED_RULE_TYPES, ELEMENT_STYLE].includes(this.type)) {
       return Promise.resolve("");
     }
 
-    const ruleBodyText = await this.getAuthoredCssText();
-    const { str: stylesheetText } = await this.sheetActor.getText();
-    const [start, end] = getSelectorOffsets(stylesheetText, this.line, this.column);
-    const selectorText = stylesheetText.substring(start, end);
+    let ruleBodyText;
+    let selectorText;
+    let text;
+
+    // For element inline styles, use the style attribute and generated unique selector.
+    if (this.type === ELEMENT_STYLE) {
+      ruleBodyText = this.rawNode.getAttribute("style");
+      selectorText = this.metadata.selector;
+    } else {
+      // Get the rule's authored text and skip any cached value.
+      ruleBodyText = await this.getAuthoredCssText(true);
+      const { str: stylesheetText } = await this.sheetActor.getText();
+      const [start, end] = getSelectorOffsets(stylesheetText, this.line, this.column);
+      selectorText = stylesheetText.substring(start, end);
+    }
 
     // CSS rule type as a string "@media", "@supports", "@keyframes", etc.
     const typeName = CSSRuleTypeName[this.type];
 
-    let text;
     // When dealing with at-rules, getSelectorOffsets() will not return the rule type.
     // We prepend it ourselves.
     if (typeName) {
       text = `${typeName}${selectorText} {${ruleBodyText}}`;
     } else {
       text = `${selectorText} {${ruleBodyText}}`;
     }
 
-    return text;
+    const prettyCSS = prettifyCSS(text);
+    return Promise.resolve(prettyCSS);
   },
 
   /**
    * Set the contents of the rule.  This rewrites the rule in the
    * stylesheet and causes it to be re-evaluated.
    *
    * @param {String} newText
    *        The new text of the rule