Bug 1664511 - [devtools] Resolve inline & inherited-inline selectors for rule-editor on demand r=nchevobbe

unique selectors for inline and inherited inline rules will be retrieved on demand.

Differential Revision: https://phabricator.services.mozilla.com/D159721

devtools/client/inspector/rules/rules.js

@@ -38,13 +38,7 @@ loader.lazyRequireGetter(
 );
 loader.lazyRequireGetter(
   this,
-  "getNodeInfo",
-  "resource://devtools/client/inspector/rules/utils/utils.js",
-  true
-);
-loader.lazyRequireGetter(
-  this,
-  "getNodeCompatibilityInfo",
+  ["getNodeInfo", "getNodeCompatibilityInfo", "getRuleFromNode"],
   "resource://devtools/client/inspector/rules/utils/utils.js",
   true
 );
@@ -417,31 +411,55 @@ CssRuleView.prototype = {
    *
    * @param {MouseEvent} event
    */
-  handleClickEvent(event) {
+  async handleClickEvent(event) {
     const target = event.target;
 
     // Handle click on the icon next to a CSS selector.
     if (target.classList.contains("js-toggle-selector-highlighter")) {
-      this.toggleSelectorHighlighter(target.dataset.selector);
       event.stopPropagation();
+      let selector = target.dataset.selector;
+      // dataset.selector will be empty for inline styles (inherited or not)
+      // Rules associated with a regular selector should have this data-attirbute
+      // set in devtools/client/inspector/rules/views/rule-editor.js
+      if (selector === "") {
+        try {
+          const rule = getRuleFromNode(target, this._elementStyle);
+          if (rule.inherited) {
+            // This is an inline style from an inherited rule. Need to resolve the
+            // unique selector from the node which this rule is inherited from.
+            selector = await rule.inherited.getUniqueSelector();
+          } else {
+            // This is an inline style from the current node.
+            selector = await this.inspector.selection.nodeFront.getUniqueSelector();
+          }
+
+          // Now that the selector was computed, we can store it in
+          // dataset.selector for subsequent usage.
+          target.dataset.selector = selector;
+        } finally {
+          // Could not resolve a unique selector for the inline style.
+        }
+      }
+
+      this.toggleSelectorHighlighter(selector);
     }
 
     // Handle click on swatches next to flex and inline-flex CSS properties
     if (target.classList.contains("js-toggle-flexbox-highlighter")) {
+      event.stopPropagation();
       this.inspector.highlighters.toggleFlexboxHighlighter(
         this.inspector.selection.nodeFront,
         "rule"
       );
-      event.stopPropagation();
     }
 
     // Handle click on swatches next to grid CSS properties
     if (target.classList.contains("js-toggle-grid-highlighter")) {
+      event.stopPropagation();
       this.inspector.highlighters.toggleGridHighlighter(
         this.inspector.selection.nodeFront,
         "rule"
       );
-      event.stopPropagation();
     }
   },
 

devtools/client/inspector/rules/test/browser_part2.ini

@@ -186,6 +186,7 @@ skip-if = ccov #Bug 1432176
 [browser_rules_selector-highlighter_03.js]
 [browser_rules_selector-highlighter_04.js]
 [browser_rules_selector-highlighter_05.js]
+[browser_rules_selector-highlighter_order.js]
 [browser_rules_selector_highlight.js]
 [browser_rules_shadowdom_slot_rules.js]
 [browser_rules_shapes-toggle_01.js]

devtools/client/inspector/rules/test/browser_rules_selector-highlighter_order.js

@@ -0,0 +1,54 @@
+/* Any copyright is dedicated to the Public Domain.
+http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+const TEST_URI = `
+<style type="text/css">
+  #rule-from-stylesheet {
+    color: red;
+  }
+</style>
+<div id=inline style="cursor:pointer">
+  A
+  <div id=inherited>B</div>
+</div>
+<div id=rule-from-stylesheet>C</a>
+`;
+
+// This test will assert that specific elements of a ruleview rule have been
+// rendered in the expected order. This is specifically done to check the fix
+// for Bug 1664511, where some elements were rendered out of order due to
+// unexpected async processing.
+add_task(async function() {
+  await addTab("data:text/html;charset=utf-8," + encodeURIComponent(TEST_URI));
+  const { inspector, view } = await openRuleView();
+
+  await selectNode("#inline", inspector);
+  checkRuleViewRuleMarkupOrder(view, "element");
+  await selectNode("#inherited", inspector);
+  checkRuleViewRuleMarkupOrder(view, "element", 1);
+  await selectNode("#rule-from-stylesheet", inspector);
+  checkRuleViewRuleMarkupOrder(view, "#rule-from-stylesheet");
+});
+
+function checkRuleViewRuleMarkupOrder(view, selectorText, index = 0) {
+  const rule = getRuleViewRule(view, selectorText, index);
+
+  // Retrieve the individual elements to assert.
+  const selectorContainer = rule.querySelector(".ruleview-selectorcontainer");
+  const highlighterIcon = rule.querySelector(".js-toggle-selector-highlighter");
+  const ruleOpenBrace = rule.querySelector(".ruleview-ruleopen");
+
+  const parentNode = selectorContainer.parentNode;
+  const childNodes = [...parentNode.childNodes];
+
+  ok(
+    childNodes.indexOf(selectorContainer) < childNodes.indexOf(highlighterIcon),
+    "Selector text is rendered before the highlighter icon"
+  );
+  ok(
+    childNodes.indexOf(highlighterIcon) < childNodes.indexOf(ruleOpenBrace),
+    "Highlighter icon is rendered before the opening brace"
+  );
+}

devtools/client/inspector/rules/test/head.js

@@ -595,8 +595,6 @@ async function clickSelectorIcon(view, selectorText, index = 0) {
   const { inspector } = view;
   const rule = getRuleViewRule(view, selectorText, index);
 
-  // The icon element is created in response to an async operation.
-  // Wait until the expected node shows up in the DOM. (Bug 1664511)
   info(`Waiting for icon to be available for selector: ${selectorText}`);
   const icon = await waitFor(() => {
     return rule.querySelector(".js-toggle-selector-highlighter");

devtools/client/inspector/rules/views/rule-editor.js

@@ -211,36 +211,23 @@ RuleEditor.prototype = {
     }
 
     if (this.rule.domRule.type !== CSSRule.KEYFRAME_RULE) {
-      // FIXME: Avoid having this as a nested async operation. (Bug 1664511)
-      (async function() {
-        let selector;
-
-        if (this.rule.domRule.selectors) {
-          // This is a "normal" rule with a selector.
-          selector = this.rule.domRule.selectors.join(", ");
-        } else if (this.rule.inherited) {
-          // This is an inline style from an inherited rule. Need to resolve the unique
-          // selector from the node which rule this is inherited from.
-          selector = await this.rule.inherited.getUniqueSelector();
-        } else {
-          // This is an inline style from the current node.
-          selector = await this.ruleView.inspector.selection.nodeFront.getUniqueSelector();
-        }
-
-        const isHighlighted = this.ruleView.isSelectorHighlighted(selector);
-        // Handling of click events is delegated to CssRuleView.handleEvent()
-        createChild(header, "span", {
-          class:
-            "ruleview-selectorhighlighter js-toggle-selector-highlighter" +
-            (isHighlighted ? " highlighted" : ""),
-          "data-selector": selector,
-          title: l10n("rule.selectorHighlighter.tooltip"),
-        });
+      let selector = "";
+      if (this.rule.domRule.selectors) {
+        // This is a "normal" rule with a selector.
+        selector = this.rule.domRule.selectors.join(", ");
+        // Otherwise, the rule is either inherited or inline, and selectors will
+        // be computed on demand when the highlighter is requested.
       }
-        .bind(this)()
-        .catch(error => {
-          console.error("Exception while getting unique selector", error);
-        }));
+
+      const isHighlighted = this.ruleView.isSelectorHighlighted(selector);
+      // Handling of click events is delegated to CssRuleView.handleEvent()
+      createChild(header, "span", {
+        class:
+          "ruleview-selectorhighlighter js-toggle-selector-highlighter" +
+          (isHighlighted ? " highlighted" : ""),
+        "data-selector": selector,
+        title: l10n("rule.selectorHighlighter.tooltip"),
+      });
     }
 
     this.openBrace = createChild(header, "span", {