Bug 1499049 - (Part 1) Log ancestor rule tree for changes to CSS declarations; r=pbro
authorRazvan Caliman <rcaliman@mozilla.com>
Wed, 24 Oct 2018 17:43:33 +0000
changeset 443116 d476c9f67f34f5e89a6600baedb64711883193dc
parent 443115 8d38810fdc7309de2520fb9b36abbb09767a4bee
child 443117 1d612a71f940facc02938a5666e68f56902e8022
push id71803
push userrcaliman@mozilla.com
push dateFri, 26 Oct 2018 11:29:10 +0000
treeherderautoland@d0c4e1db970e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspbro
bugs1499049
milestone65.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 1499049 - (Part 1) Log ancestor rule tree for changes to CSS declarations; r=pbro ⚠️ **To build locally, this change series depends on the [change series](https://phabricator.services.mozilla.com/D4399) which adds the ChangesActor**. 🏋️‍♂️ **To test hands-on, you can download a [custom macOS build](https://queue.taskcluster.net/v1/task/HIiZcwLXTuuSYYjfwEDmmA/runs/0/artifacts/public/build/target.dmg) (updated Wed, Oct 24) which includes both change series.** - Introduce ancestorRules getter to StyleRuleActor to get a flattened rule tree with the ancestors of the current rule; - Introduce CSSRuleTypeName to css-logic helpers to map between CSS rule type and human-readable name; - Log rule index position with each CSS declaration change to help differentiate between changes to rules with identical selectors at the same level of nesting. Differential Revision: https://phabricator.services.mozilla.com/D8718
devtools/server/actors/styles.js
devtools/shared/inspector/css-logic.js
--- a/devtools/server/actors/styles.js
+++ b/devtools/server/actors/styles.js
@@ -22,17 +22,20 @@ loader.lazyRequireGetter(this, "getDefin
 loader.lazyRequireGetter(this, "isCssPropertyKnown",
   "devtools/server/actors/css-properties", true);
 loader.lazyRequireGetter(this, "parseNamedDeclarations",
   "devtools/shared/css/parsing-utils", 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, "findCssSelector",
+  "devtools/shared/inspector/css-logic", true);
+loader.lazyRequireGetter(this, "CSSRuleTypeName",
+  "devtools/shared/inspector/css-logic", true);
 
 loader.lazyGetter(this, "PSEUDO_ELEMENTS", () => {
   return InspectorUtils.getCSSPseudoElementNames();
 });
 loader.lazyGetter(this, "FONT_VARIATIONS_ENABLED", () => {
   return Services.prefs.getBoolPref("layout.css.font-variations.enabled");
 });
 
@@ -981,23 +984,23 @@ var StyleRuleActor = protocol.ActorClass
     // Parsed CSS declarations from this.form().declarations used to check CSS property
     // names and values before tracking changes. Using cached values instead of accessing
     // this.form().declarations on demand because that would cause needless re-parsing.
     this._declarations = [];
 
     if (CSSRule.isInstance(item)) {
       this.type = item.type;
       this.rawRule = item;
+      this._computeRuleIndex();
       if ((this.type === CSSRule.STYLE_RULE ||
            this.type === CSSRule.KEYFRAME_RULE) &&
           this.rawRule.parentStyleSheet) {
         this.line = InspectorUtils.getRelativeRuleLine(this.rawRule);
         this.column = InspectorUtils.getRuleColumn(this.rawRule);
         this._parentSheet = this.rawRule.parentStyleSheet;
-        this._computeRuleIndex();
         this.sheetActor = this.pageStyle._sheetRef(this._parentSheet);
         this.sheetActor.on("style-applied", this._onStyleApplied);
       }
     } else {
       // Fake a rule
       this.type = ELEMENT_STYLE;
       this.rawNode = item;
       this.rawRule = {
@@ -1044,16 +1047,35 @@ var StyleRuleActor = protocol.ActorClass
             // https://bugzilla.mozilla.org/show_bug.cgi?id=1224121
             !this.sheetActor.hasRulesModifiedByCSSOM() &&
             // Special case about:PreferenceStyleSheet, as it is generated on
             // the fly and the URI is not registered with the about:handler
             // https://bugzilla.mozilla.org/show_bug.cgi?id=935803#c37
             this._parentSheet.href !== "about:PreferenceStyleSheet");
   },
 
+  /**
+   * Return an array with StyleRuleActor instances for each of this rule's ancestor rules
+   * (@media, @supports, @keyframes, etc) obtained by recursively reading rule.parentRule.
+   * If the rule has no ancestors, return an empty array.
+   *
+   * @return {Array}
+   */
+  get ancestorRules() {
+    const ancestors = [];
+    let rule = this.rawRule;
+
+    while (rule.parentRule) {
+      ancestors.push(this.pageStyle._styleRef(rule.parentRule));
+      rule = rule.parentRule;
+    }
+
+    return ancestors;
+  },
+
   getDocument: function(sheet) {
     if (sheet.ownerNode) {
       return sheet.ownerNode.nodeType == sheet.ownerNode.DOCUMENT_NODE ?
              sheet.ownerNode : sheet.ownerNode.ownerDocument;
     } else if (sheet.parentStyleSheet) {
       return this.getDocument(sheet.parentStyleSheet);
     }
     throw (new Error("Failed trying to get the document of an invalid stylesheet"));
@@ -1181,20 +1203,20 @@ var StyleRuleActor = protocol.ActorClass
    * a given CSS rule in its parent.  A vector is used to support
    * nested rules.
    */
   _computeRuleIndex: function() {
     let rule = this.rawRule;
     const result = [];
 
     while (rule) {
-      let cssRules;
+      let cssRules = [];
       if (rule.parentRule) {
         cssRules = rule.parentRule.cssRules;
-      } else {
+      } else if (rule.parentStyleSheet) {
         cssRules = rule.parentStyleSheet.cssRules;
       }
 
       let found = false;
       for (let i = 0; i < cssRules.length; i++) {
         if (rule === cssRules.item(i)) {
           found = true;
           result.unshift(i);
@@ -1471,29 +1493,63 @@ var StyleRuleActor = protocol.ActorClass
       ? this._declarations[change.index].value
       : null;
     const prevName = this._declarations[change.index]
       ? this._declarations[change.index].name
       : null;
 
     // Metadata about a change.
     const data = {};
-    data.type = change.type;
-    data.selector = this.rawRule.selectorText;
+    // Collect information about the rule's ancestors (@media, @supports, @keyframes).
+    // Used to show context for this change in the UI and to match the rule for undo/redo.
+    data.ancestors = this.ancestorRules.map(rule => {
+      return {
+        // Rule type as number defined by CSSRule.type (ex: 4, 7, 12)
+        // @see https://developer.mozilla.org/en-US/docs/Web/API/CSSRule
+        type: rule.rawRule.type,
+        // Rule type as human-readable string (ex: "@media", "@supports", "@keyframes")
+        typeName: CSSRuleTypeName[rule.rawRule.type],
+        // Conditions of @media and @supports rules (ex: "min-width: 1em")
+        conditionText: rule.rawRule.conditionText,
+        // Name of @keyframes rule; refrenced by the animation-name CSS property.
+        name: rule.rawRule.name,
+        // Selector of individual @keyframe rule within a @keyframes rule (ex: 0%, 100%).
+        keyText: rule.rawRule.keyText,
+        // Array with the indexes of this rule and its ancestors within the CSS rule tree.
+        ruleIndex: rule._ruleIndex,
+      };
+    });
 
-    // For inline style changes, generate a unique selector and pass the node tag.
+    // For changes in element style attributes, generate a unique selector.
     if (this.type === ELEMENT_STYLE) {
-      data.tag = this.rawNode.tagName;
-      data.href = "inline";
       // findCssSelector() fails on XUL documents. Catch and silently ignore that error.
       try {
         data.selector = findCssSelector(this.rawNode);
       } catch (err) {}
+
+      data.source = {
+        type: "element",
+        // Used to differentiate between elements which match the same generated selector
+        // but live in different documents (ex: host document and iframe).
+        href: this.rawNode.baseURI,
+        // Element style attributes don't have a rule index; use the generated selector.
+        index: data.selector,
+      };
+      data.ruleIndex = 0;
     } else {
-      data.href = this._parentSheet.href || "inline stylesheet";
+      data.selector = (this.type === CSSRule.KEYFRAME_RULE)
+        ? this.rawRule.keyText
+        : this.rawRule.selectorText;
+      data.source = {
+        type: "stylesheet",
+        href: this.sheetActor.href,
+        index: this.sheetActor.styleSheetIndex,
+      };
+      // Used to differentiate between changes to rules with identical selectors.
+      data.ruleIndex = this._ruleIndex;
     }
 
     switch (change.type) {
       case "set":
         // If `change.newName` is defined, use it because the property is being renamed.
         // Otherwise, a new declaration is being created or the value of an existing
         // declaration is being updated. In that case, use the provided `change.name`.
         const name = change.newName ? change.newName : change.name;
@@ -1508,24 +1564,16 @@ var StyleRuleActor = protocol.ActorClass
         break;
 
       case "remove":
         data.add = null;
         data.remove = { property: change.name, value: prevValue };
         break;
     }
 
-    // Do not track non-changes. This can occur when typing a value in the Rule view
-    // inline editor, then committing it by pressing the Enter key.
-    if (data.add && data.remove &&
-        data.add.property === data.remove.property &&
-        data.add.value === data.remove.value) {
-      return;
-    }
-
     TrackChangeEmitter.trackChange(data);
   },
 
   /**
    * Calls modifySelector2() which needs to be kept around for backwards compatibility.
    * TODO: Once Firefox 64 is no longer supported, inline that mehtod's content,
    * then remove its definition from this file and from specs/styles.js
    */
--- a/devtools/shared/inspector/css-logic.js
+++ b/devtools/shared/inspector/css-logic.js
@@ -78,16 +78,36 @@ exports.STATUS = {
   BEST: 3,
   MATCHED: 2,
   PARENT_MATCH: 1,
   UNMATCHED: 0,
   UNKNOWN: -1,
 };
 
 /**
+ * Mapping of CSSRule type value to CSSRule type name.
+ * @see https://developer.mozilla.org/en-US/docs/Web/API/CSSRule
+ */
+exports.CSSRuleTypeName = {
+  1: "", // Regular CSS style rule has no name
+  3: "@import",
+  4: "@media",
+  5: "@font-face",
+  6: "@page",
+  7: "@keyframes",
+  8: "@keyframe",
+  10: "@namespace",
+  11: "@counter-style",
+  12: "@supports",
+  13: "@document",
+  14: "@font-feature-values",
+  15: "@viewport",
+};
+
+/**
  * Lookup a l10n string in the shared styleinspector string bundle.
  *
  * @param {String} name
  *        The key to lookup.
  * @returns {String} A localized version of the given key.
  */
 exports.l10n = name => styleInspectorL10N.getStr(name);