Bug 1626234 - [devtools] Display the computed value of custom property in var() tooltip. r=devtools-reviewers,bomsy.

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

devtools/client/inspector/rules/models/element-style.js

@@ -418,7 +418,10 @@ class ElementStyle {
             !computedProp.textProp.invisible
           ) {
             if (!isPropInStartingStyle) {
-              variables.set(computedProp.name, computedProp.value);
+              variables.set(computedProp.name, {
+                declarationValue: computedProp.value,
+                computedValue: computedProp.textProp.getVariableComputedValue(),
+              });
             } else {
               startingStyleVariables.set(computedProp.name, computedProp.value);
             }
@@ -937,7 +940,9 @@ class ElementStyle {
     if (variables?.has(name)) {
       // XXX Check what to do in case the value doesn't match the registered property syntax.
       // Will be handled in Bug 1866712
-      data.value = variables.get(name);
+      const { declarationValue, computedValue } = variables.get(name);
+      data.value = declarationValue;
+      data.computedValue = computedValue;
     }
     if (startingStyleVariables?.has(name)) {
       data.startingStyle = startingStyleVariables.get(name);
@@ -959,7 +964,11 @@ class ElementStyle {
    *                              value if the property is not defined)
    */
   getAllCustomProperties(pseudo = "") {
-    let customProperties = this.variablesMap.get(pseudo);
+    const customProperties = new Map();
+    for (const [key, { declarationValue }] of this.variablesMap.get(pseudo)) {
+      customProperties.set(key, declarationValue);
+    }
+
     const startingStyleCustomProperties =
       this.startingStyleVariablesMap.get(pseudo);
 
@@ -975,19 +984,11 @@ class ElementStyle {
       return customProperties;
     }
 
-    let newMapCreated = false;
-
     if (startingStyleCustomProperties) {
       for (const [name, value] of startingStyleCustomProperties) {
         // Only set the starting style property if it's not defined (i.e. not in the "main"
         // variable map)
         if (!customProperties.has(name)) {
-          // Since we want to return starting style variables, we need to create a new Map
-          // to not modify the one in the main map.
-          if (!newMapCreated) {
-            customProperties = new Map(customProperties);
-            newMapCreated = true;
-          }
           customProperties.set(name, value);
         }
       }
@@ -997,12 +998,6 @@ class ElementStyle {
       for (const [name, propertyDefinition] of registeredPropertiesMap) {
         // Only set the registered property if it's not defined (i.e. not in the variable map)
         if (!customProperties.has(name)) {
-          // Since we want to return registered property, we need to create a new Map
-          // to not modify the one in the variable map.
-          if (!newMapCreated) {
-            customProperties = new Map(customProperties);
-            newMapCreated = true;
-          }
           customProperties.set(name, propertyDefinition.initialValue);
         }
       }

devtools/client/inspector/rules/models/text-property.js

@@ -409,6 +409,23 @@ class TextProperty {
     return declaration.invalidAtComputedValueTime;
   }
 
+  /**
+   * Get the associated CSS variable computed value.
+   *
+   * @return {String}
+   */
+  getVariableComputedValue() {
+    const declaration = this.#getDomRuleDeclaration();
+    // When adding a new property in the rule-view, the TextProperty object is
+    // created right away before the rule gets updated on the server, so we're
+    // not going to find the corresponding declaration object yet. Default to null.
+    if (!declaration || !declaration.isCustomProperty) {
+      return null;
+    }
+
+    return declaration.computedValue;
+  }
+
   /**
    * Returns the expected syntax for this property.
    * For now, it's only sent from the server for invalid at computed-value time declarations.

devtools/client/inspector/rules/test/browser_rules_at_starting-style.js

@@ -277,6 +277,8 @@ add_task(async function () {
     "color",
     {
       header: "--my-color = white",
+      // Computed value isn't displayed when it's the same as we put in the header
+      computed: null,
       // The starting-style value is displayed in the tooltip
       startingStyle: "--my-color = black",
     }
@@ -292,6 +294,8 @@ add_task(async function () {
     {
       // The displayed value is the one set in the starting-style rule
       header: "--my-color = black",
+      // Computed value isn't displayed in starting-style rule
+      computed: null,
       // The starting-style section is not displayed when hovering starting-style rule
       startingStyle: null,
     }
@@ -310,6 +314,7 @@ add_task(async function () {
     "--check-my-overridden-color",
     {
       header: "--my-overridden-color = white",
+      computed: "white",
       // The starting-style rule is overridden, so we don't show a starting-style section in the tooltip
       startingStyle: null,
     }
@@ -321,6 +326,8 @@ add_task(async function () {
     {
       // the value is the one from the regular rule, not the one from the starting-style rule
       header: "--my-overridden-color = white",
+      // Computed value isn't displayed in starting-style rule
+      computed: null,
       startingStyle: null,
     }
   );
@@ -334,6 +341,7 @@ add_task(async function () {
     "--check-my-registered-color",
     {
       header: "--my-registered-color = white",
+      computed: "rgb(255, 255, 255)",
       // The starting-style value is displayed in the tooltip
       startingStyle: "--my-registered-color = black",
       // registered property data is displayed
@@ -352,6 +360,8 @@ add_task(async function () {
     {
       // The displayed value is the one set in the starting-style rule
       header: "--my-registered-color = black",
+      // Computed value isn't displayed in starting-style rule
+      computed: null,
       // The starting-style section is not displayed when hovering starting-style rule
       startingStyle: null,
       // registered property data is displayed

devtools/client/inspector/rules/test/browser_rules_registered-custom-properties.js

@@ -160,6 +160,7 @@ add_task(async function () {
   await assertVariableTooltipForProperty(view, "h1", "color", {
     // The variable value is the value set in the main selector, since the variable does inherit
     header: `--css-inherit = ${CSS_INHERIT_MAIN_VALUE}`,
+    computed: "rgb(255, 0, 0)",
     registeredProperty: [
       `syntax:"<color>"`,
       `inherits:true`,
@@ -201,6 +202,8 @@ add_task(async function () {
     // The variable value is the value set in the main selector, since the variable does inherit
     {
       header: `--js-inherit = ${JS_INHERIT_MAIN_VALUE}`,
+      // Computed value isn't displayed when it's the same as we put in the header
+      computed: null,
       registeredProperty: [`syntax:"*"`, `inherits:true`],
     }
   );
@@ -308,6 +311,7 @@ add_task(async function () {
   // The var() tooltip should indicate that the property isn't set anymore
   await assertVariableTooltipForProperty(view, "h1", "caret-color", {
     header: `--css-dynamic-registered is not set`,
+    isMatched: false,
   });
 
   info(

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

@@ -22,51 +22,34 @@ add_task(async function () {
     "div",
     "color"
   ).valueSpan.querySelector(".ruleview-unmatched");
-  const setColor = unsetColor.previousElementSibling;
   is(unsetColor.textContent, " red", "red is unmatched in color");
-  is(setColor.textContent, "--color", "--color is not set correctly");
-  is(
-    setColor.dataset.variable,
-    "--color = chartreuse",
-    "--color's dataset.variable is not set correctly"
-  );
-  let previewTooltip = await assertShowPreviewTooltip(view, setColor);
-  await assertTooltipHiddenOnMouseOut(previewTooltip, setColor);
 
-  ok(
-    previewTooltip.panel.textContent.includes("--color = chartreuse"),
-    "CSS variable preview tooltip shows the expected CSS variable"
-  );
+  await assertVariableTooltipForProperty(view, "div", "color", {
+    header: "--color = chartreuse",
+    // Computed value isn't displayed when it's the same as we put in the header
+    computed: null,
+  });
 
-  const unsetVar = getRuleViewProperty(
-    view,
-    "div",
-    "background-color"
-  ).valueSpan.querySelector(".ruleview-unmatched");
-  const setVar = unsetVar.nextElementSibling;
-  const setVarName = setVar.querySelector(".ruleview-variable");
-  is(
-    unsetVar.textContent,
-    "--not-set",
-    "--not-set is unmatched in background-color"
-  );
-  is(setVar.textContent, " var(--bg)", "var(--bg) parsed incorrectly");
-  is(setVarName.textContent, "--bg", "--bg is not set correctly");
-  is(
-    setVarName.dataset.variable,
-    "--bg = seagreen",
-    "--bg's dataset.variable is not set correctly"
-  );
-  previewTooltip = await assertShowPreviewTooltip(view, setVarName);
+  await assertVariableTooltipForProperty(view, "div", "background-color", {
+    index: 0,
+    header: "--not-set is not set",
+    isMatched: false,
+  });
 
-  ok(
-    !previewTooltip.panel.textContent.includes("--color = chartreuse"),
-    "CSS variable preview tooltip no longer shows the previous CSS variable"
-  );
-  ok(
-    previewTooltip.panel.textContent.includes("--bg = seagreen"),
-    "CSS variable preview tooltip shows the new CSS variable"
-  );
+  await assertVariableTooltipForProperty(view, "div", "background-color", {
+    index: 1,
+    header: "--bg = seagreen",
+    // Computed value isn't displayed when it's the same as we put in the header
+    computed: null,
+  });
 
-  await assertTooltipHiddenOnMouseOut(previewTooltip, setVarName);
+  await assertVariableTooltipForProperty(view, "div", "outline-color", {
+    header: "--nested = var(--color)",
+    computed: "chartreuse",
+  });
+
+  await assertVariableTooltipForProperty(view, "div", "border-color", {
+    header: "--nested-with-function = var(--theme-color)",
+    computed: "light-dark(chartreuse, seagreen)",
+  });
 });

devtools/client/inspector/rules/test/doc_variables_1.html

@@ -8,12 +8,17 @@
     * {
       --color: tomato;
       --bg: violet;
+      --nested: var(--color);
+      --theme-color: light-dark(var(--color), var(--bg));
+      --nested-with-function: var(--theme-color);
     }
 
     div {
       --color: chartreuse;
       color: var(--color, red);
       background-color: var(--not-set, var(--bg));
+      outline-color: var(--nested);
+      border-color: var(--nested-with-function);
     }
   </style>
 </head>

devtools/client/inspector/rules/utils/utils.js

@@ -166,6 +166,7 @@ function getNodeInfo(node, elementStyle) {
       sheetHref: rule.domRule.href,
       textProperty: declaration,
       variable: node.dataset.variable,
+      variableComputed: node.dataset.variableComputed,
       startingStyleVariable: node.dataset.startingStyleVariable,
       registeredProperty: {
         initialValue: node.dataset.registeredPropertyInitialValue,

devtools/client/inspector/shared/tooltips-overlay.js

@@ -347,10 +347,15 @@ TooltipsOverlay.prototype = {
       type === TOOLTIP_VARIABLE_TYPE &&
       nodeInfo.value.value.startsWith("--")
     ) {
-      const { variable, registeredProperty, startingStyleVariable } =
-        nodeInfo.value;
+      const {
+        variable,
+        registeredProperty,
+        startingStyleVariable,
+        variableComputed,
+      } = nodeInfo.value;
       await this._setVariablePreviewTooltip({
         topSectionText: variable,
+        computed: variableComputed,
         registeredProperty,
         startingStyle: startingStyleVariable,
       });

devtools/client/inspector/test/head.js

@@ -983,7 +983,7 @@ async function assertShowPreviewTooltip(view, target) {
  *        The DOM Element on which a tooltip should appear
  */
 async function assertTooltipHiddenOnMouseOut(tooltip, target) {
-  // The tooltip actually relies on mousemove events to check if it sould be hidden.
+  // The tooltip actually relies on mousemove events to check if it should be hidden.
   const mouseEvent = new target.ownerDocument.defaultView.MouseEvent(
     "mousemove",
     {
@@ -1008,6 +1008,12 @@ async function assertTooltipHiddenOnMouseOut(tooltip, target) {
  * @param {String} tooltipExpected.header: The text that is displayed in the top section
  *        (might be the only section when the variable is not a registered property and
  *        there is no starting-style).
+ * @param {String} tooltipExpected.computed: The text that is displayed in the computed value section.
+ * @param {Integer} tooltipExpected.index: The index in the property value for the variable
+ *        element we want to check. Defaults to 0 so we can quickly check values when only
+ *        one variable is used.
+ * @param {Boolean} tooltipExpected.isMatched: Is the element matched or unmatched, defaults
+ *        to true.
  * @param {String} tooltipExpected.startingStyle: The text that is displayed in the starting-style
  *        section. Pass undefined if the tooltip isn't supposed to have a `@starting-style` section.
  * @param {Array<String>} tooltipExpected.registeredProperty: Array of the registered property
@@ -1018,19 +1024,42 @@ async function assertVariableTooltipForProperty(
   view,
   ruleSelector,
   propertyName,
-  { header, registeredProperty, startingStyle }
+  {
+    computed,
+    header,
+    index = 0,
+    isMatched = true,
+    registeredProperty,
+    startingStyle,
+  }
 ) {
   // retrieve tooltip target
-  const variableEl = await waitFor(() =>
+  const variableEl = await waitFor(
+    () =>
       getRuleViewProperty(
         view,
         ruleSelector,
         propertyName
-    ).valueSpan.querySelector(".ruleview-variable,.ruleview-unmatched")
+      ).valueSpan.querySelectorAll(".ruleview-variable,.ruleview-unmatched")[
+        index
+      ]
   );
 
+  if (isMatched) {
+    ok(
+      !variableEl.classList.contains("ruleview-unmatched"),
+      `CSS variable #${index} for ${propertyName} in ${ruleSelector} is matched`
+    );
+  } else {
+    ok(
+      variableEl.classList.contains("ruleview-unmatched"),
+      `CSS variable #${index} for ${propertyName} in ${ruleSelector} is unmatched`
+    );
+  }
+
   const previewTooltip = await assertShowPreviewTooltip(view, variableEl);
   const valueEl = previewTooltip.panel.querySelector(".variable-value");
+  const computedValueEl = previewTooltip.panel.querySelector(".computed div");
   const startingStyleEl = previewTooltip.panel.querySelector(
     ".starting-style div"
   );
@@ -1038,22 +1067,36 @@ async function assertVariableTooltipForProperty(
     ".registered-property dl"
   );
   is(
-    valueEl.textContent,
+    valueEl?.textContent,
     header,
-    `CSS variable preview tooltip has expected header text for ${propertyName} in ${ruleSelector}`
+    `CSS variable #${index} preview tooltip has expected header text for ${propertyName} in ${ruleSelector}`
   );
 
+  if (typeof computed !== "string") {
+    is(
+      computedValueEl,
+      null,
+      `CSS variable #${index} preview tooltip doesn't have computed value section for ${propertyName} in ${ruleSelector}`
+    );
+  } else {
+    is(
+      computedValueEl?.innerText,
+      computed,
+      `CSS variable #${index} preview tooltip has expected computed value section for ${propertyName} in ${ruleSelector}`
+    );
+  }
+
   if (!registeredProperty) {
     is(
       registeredPropertyEl,
       null,
-      `CSS variable preview tooltip doesn't have registered property section for ${propertyName} in ${ruleSelector}`
+      `CSS variable #${index} preview tooltip doesn't have registered property section for ${propertyName} in ${ruleSelector}`
     );
   } else {
     is(
-      registeredPropertyEl.innerText,
+      registeredPropertyEl?.innerText,
       registeredProperty.join("\n"),
-      `CSS variable preview tooltip has expected registered property section for ${propertyName} in ${ruleSelector}`
+      `CSS variable #${index} preview tooltip has expected registered property section for ${propertyName} in ${ruleSelector}`
     );
   }
 
@@ -1061,13 +1104,13 @@ async function assertVariableTooltipForProperty(
     is(
       startingStyleEl,
       null,
-      `CSS variable preview tooltip doesn't have a starting-style section for ${propertyName} in ${ruleSelector}`
+      `CSS variable #${index} preview tooltip doesn't have a starting-style section for ${propertyName} in ${ruleSelector}`
     );
   } else {
     is(
-      startingStyleEl.innerText,
+      startingStyleEl?.innerText,
       startingStyle,
-      `CSS variable preview tooltip has expected starting-style section for ${propertyName} in ${ruleSelector}`
+      `CSS variable #${index} preview tooltip has expected starting-style section for ${propertyName} in ${ruleSelector}`
     );
   }
 

devtools/client/shared/output-parser.js

@@ -301,6 +301,7 @@ class OutputParser {
     let varData;
     let varFallbackValue;
     let varSubsitutedValue;
+    let varComputedValue;
 
     // Get the variable value if it is in use.
     if (tokens && tokens.length === 1) {
@@ -321,6 +322,8 @@ class OutputParser {
       varSubsitutedValue = options.inStartingStyleRule
         ? varStartingStyleValue
         : varValue;
+
+      varComputedValue = varData.computedValue;
     }
 
     // Get the variable name.
@@ -337,6 +340,16 @@ class OutputParser {
       firstOpts.class = options.matchedVariableClass;
       secondOpts.class = options.unmatchedClass;
 
+      // Display computed value when it exists, is different from the substituted value
+      // we computed, and we're not inside a starting-style rule
+      if (
+        !options.inStartingStyleRule &&
+        typeof varComputedValue === "string" &&
+        varComputedValue !== varSubsitutedValue
+      ) {
+        firstOpts["data-variable-computed"] = varComputedValue;
+      }
+
       // Display starting-style value when not in a starting style rule
       if (
         !options.inStartingStyleRule &&

devtools/client/shared/test/browser_outputparser.js

@@ -635,6 +635,17 @@ function testParseVariable(doc, parser) {
           "</span>" +
         "</span>",
     },
+    {
+      text: "var(--seen)",
+      variables: {
+        "--seen": { value: "var(--base)", computedValue: "1em" },
+      },
+      expected:
+        // prettier-ignore
+        "<span>var(" +
+          '<span data-variable="--seen = var(--base)" data-variable-computed="1em">--seen</span>)' +
+        "</span>",
+    },
     {
       text: "var(--not-seen)",
       variables: {},

devtools/client/shared/widgets/tooltip/VariableTooltipHelper.js

@@ -17,6 +17,8 @@ const XHTML_NS = "http://www.w3.org/1999/xhtml";
  * @param  {Object} params
  * @param  {String} params.topSectionText
  *         Text to display in the top section of tooltip (e.g. "--x = blue" or "--x is not defined").
+ * @param  {String} params.computed
+ *         The computed value for the variable.
  * @param  {Object} params.registeredProperty
  *         Contains the registered property data, if the variable was registered (@property or CSS.registerProperty)
  * @param  {String} params.registeredProperty.syntax
@@ -31,7 +33,7 @@ const XHTML_NS = "http://www.w3.org/1999/xhtml";
 function setVariableTooltip(
   tooltip,
   doc,
-  { topSectionText, registeredProperty, startingStyle }
+  { computed, topSectionText, registeredProperty, startingStyle }
 ) {
   // Create tooltip content
   const div = doc.createElementNS(XHTML_NS, "div");
@@ -42,7 +44,19 @@ function setVariableTooltip(
   valueEl.append(doc.createTextNode(topSectionText));
   div.appendChild(valueEl);
 
-  // A registered property always have a non-falsy syntax
+  if (typeof computed !== "undefined") {
+    const section = doc.createElementNS(XHTML_NS, "section");
+    section.classList.add("computed", "variable-tooltip-section");
+
+    const h2 = doc.createElementNS(XHTML_NS, "h2");
+    h2.append(doc.createTextNode("computed value"));
+    const computedValueEl = doc.createElementNS(XHTML_NS, "div");
+    computedValueEl.append(doc.createTextNode(computed));
+    section.append(h2, computedValueEl);
+
+    div.appendChild(section);
+  }
+
   if (typeof startingStyle !== "undefined") {
     const section = doc.createElementNS(XHTML_NS, "section");
     section.classList.add("starting-style", "variable-tooltip-section");

devtools/server/actors/style-rule.js

@@ -432,6 +432,7 @@ class StyleRuleActor extends Actor {
 
         if (SharedCssLogic.isCssVariable(decl.name)) {
           decl.isCustomProperty = true;
+          decl.computedValue = style.getPropertyValue(decl.name);
 
           // If the variable is a registered property, we check if the variable is
           // invalid at computed-value time (e.g. if the declaration value matches