Bug 1699648 - [devtools] Use the proper principal to fetch the text of imported stylesheets r=nchevobbe

Imported stylesheets don't have an ownerNode property, it needs to be retrieved on the parentStylesheet (potentially recursively). Add a helper to resolve the ownerNode and use it retrieve the correct principal to fetch the stylesheet. Add a test case simulating a stylesheet update. Differential Revision: https://phabricator.services.mozilla.com/D109629
2021-03-25 18:30:09 +00:00 · 2021-03-25 18:30:09 +00:00 · 585c3dd0d5
--- a/devtools/client/inspector/rules/test/browser_part2.ini
+++ b/devtools/client/inspector/rules/test/browser_part2.ini
@ -21,6 +21,7 @@ support-files =
  doc_print_media_simulation.html
  doc_pseudoelement.html
  doc_ruleLineNumbers.html
+  doc_rules_imported_stylesheet_edit.html
  doc_sourcemaps.css
  doc_sourcemaps.css.map
  doc_sourcemaps.html
@ -39,6 +40,7 @@ support-files =
  doc_visited_in_media_query.html
  doc_visited_with_style_attribute.html
  head.js
+  sjs_imported_stylesheet_edit.sjs
  !/devtools/client/inspector/test/head.js
  !/devtools/client/inspector/test/shared-head.js
  !/devtools/client/shared/test/shared-head.js
@ -83,6 +85,7 @@ skip-if = os == 'linux' # focusEditableField times out consistently on linux.
 [browser_rules_highlight-element-rule.js]
 [browser_rules_highlight-property.js]
 [browser_rules_highlight-used-fonts.js]
+[browser_rules_imported_stylesheet_edit.js]
 [browser_rules_inactive_css_display-justify.js]
 [browser_rules_inactive_css_flexbox.js]
 [browser_rules_inactive_css_grid.js]
--- a/devtools/client/inspector/rules/test/browser_rules_imported_stylesheet_edit.js
+++ b/devtools/client/inspector/rules/test/browser_rules_imported_stylesheet_edit.js
@ -0,0 +1,46 @@
+/* Any copyright is dedicated to the Public Domain.
+ http://creativecommons.org/publicdomain/zero/1.0/ */
+"use strict";
+
+const TEST_URI = URL_ROOT + "doc_rules_imported_stylesheet_edit.html";
+const SJS_URI = URL_ROOT + "sjs_imported_stylesheet_edit.sjs";
+/**
+ * Test that imported stylesheets are correctly handled by the inspector after
+ * being updated.
+ * The inspector used to retrieve an outdated version of the stylesheet text,
+ * which triggered many issues: outdated values, blank panels etc...
+ *
+ * This test involves an imported CSS which is generated by a sjs file.
+ * Using sjs here allows us to simulate an "update" of a stylesheet while still
+ * fetching the same URL, which closely matches what a developer would experience
+ * when manually editing a stylesheet in an IDE before reloading a page.
+ */
+add_task(async function() {
+  info("Call `?setup` on the test sjs");
+  await fetch(SJS_URI + "?setup");
+
+  info("Add the test tab, open the rule-view and select the test node");
+  await addTab(TEST_URI);
+
+  const { inspector, view } = await openRuleView();
+
+  await selectNode("div", inspector);
+  const redColorProp = getTextProperty(view, 1, { color: "red" });
+  ok(redColorProp, "RuleView displays a color:red property");
+
+  // The "?update-stylesheet" call will change the CSS returned by sjs_imported_stylesheet_edit.sjs:
+  // - some rules are added before the matching `div {}` rule
+  // - the value of the `color` property changes
+  info("Call `?update-stylesheet` on the test sjs");
+  await fetch(SJS_URI + "?update-stylesheet");
+
+  info("Reload the page to restore the initial state");
+  await navigateTo(TEST_URI);
+
+  info("Wait until a rule is displayed at index 1");
+  await waitFor(() => view.element.children[1]);
+
+  info("Check that the displayed rule has been correctly updated.");
+  const goldColorProp = getTextProperty(view, 1, { color: "gold" });
+  ok(goldColorProp, "RuleView displays a color:gold property");
+});
--- a/devtools/client/inspector/rules/test/doc_rules_imported_stylesheet_edit.html
+++ b/devtools/client/inspector/rules/test/doc_rules_imported_stylesheet_edit.html
@ -0,0 +1,4 @@
+<style type="text/css">
+  @import url("./sjs_imported_stylesheet_edit.sjs");
+</style>
+<body><div></div></body>
--- a/devtools/client/inspector/rules/test/sjs_imported_stylesheet_edit.sjs
+++ b/devtools/client/inspector/rules/test/sjs_imported_stylesheet_edit.sjs
@ -0,0 +1,53 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at <http://mozilla.org/MPL/2.0/>. */
+
+"use strict";
+const INITIAL_CONTENT = `
+div {
+  color: red
+}
+`;
+
+const UPDATED_CONTENT = `
+span {
+  color: green;
+}
+
+a {
+  color: blue;
+}
+
+div {
+  color: gold;
+}
+`;
+
+/**
+ * This sjs file supports three endpoint:
+ * - "sjs_imported_stylesheet_edit.sjs" -> will return a text/css content which
+ *   will be either INITIAL_CONTENT or UPDATED_CONTENT. Initially will return
+ *   INITIAL_CONTENT.
+ * - "sjs_imported_stylesheet_edit.sjs?update-stylesheet" -> will update an
+ *   internal flag. After calling this URL, the regular endpoint will return
+ *   UPDATED_CONTENT instead of INITIAL_CONTENT
+ * - "sjs_imported_stylesheet_edit.sjs?setup" -> set the internal flag to its
+ *   default value. Should be called at the beginning of every test to avoid
+ *   side effects.
+ */
+function handleRequest(request, response) {
+  const { queryString } = request;
+  if (queryString === "setup") {
+    setState("serve-updated-content", "false");
+    response.setHeader("Content-Type", "text/html");
+    response.write("OK");
+  } else if (queryString === "update-stylesheet") {
+    setState("serve-updated-content", "true");
+    response.setHeader("Content-Type", "text/html");
+    response.write("OK");
+  } else {
+    response.setHeader("Content-Type", "text/css");
+    const shouldServeUpdatedCSS = getState("serve-updated-content") == "true";
+    response.write(shouldServeUpdatedCSS ? UPDATED_CONTENT : INITIAL_CONTENT);
+  }
+}
--- a/devtools/server/actors/resources/stylesheets.js
+++ b/devtools/server/actors/resources/stylesheets.js
@ -35,7 +35,7 @@ loader.lazyRequireGetter(
 );
 loader.lazyRequireGetter(
  this,
-  ["UPDATE_GENERAL", "UPDATE_PRESERVING_RULES"],
+  ["getSheetOwnerNode", "UPDATE_GENERAL", "UPDATE_PRESERVING_RULES"],
  "devtools/server/actors/style-sheet",
  true
 );
@ -342,10 +342,11 @@ class StyleSheetWatcher {
    const excludedProtocolsRe = /^(chrome|file|resource|moz-extension):\/\//;
    if (!excludedProtocolsRe.test(href)) {
      // Stylesheets using other protocols should use the content principal.
-      if (styleSheet.ownerNode) {
+      const ownerNode = getSheetOwnerNode(styleSheet);
+      if (ownerNode) {
        // eslint-disable-next-line mozilla/use-ownerGlobal
-        options.window = styleSheet.ownerNode.ownerDocument.defaultView;
-        options.principal = styleSheet.ownerNode.ownerDocument.nodePrincipal;
+        options.window = ownerNode.ownerDocument.defaultView;
+        options.principal = ownerNode.ownerDocument.nodePrincipal;
      }
    }

@ -499,18 +500,12 @@ class StyleSheetWatcher {
  }

  _getSourcemapBaseURL(styleSheet) {
-    // When the style is imported, `styleSheet.ownerNode` is null,
-    // so retrieve the topmost parent style sheet which has an ownerNode
-    let parentStyleSheet = styleSheet;
-    while (parentStyleSheet.parentStyleSheet) {
-      parentStyleSheet = parentStyleSheet.parentStyleSheet;
-    }
-
    // When the style is injected via nsIDOMWindowUtils.loadSheet, even
    // the parent style sheet has no owner, so default back to target actor
    // document
-    const ownerDocument = parentStyleSheet.ownerNode
-      ? parentStyleSheet.ownerNode.ownerDocument
+    const ownerNode = getSheetOwnerNode(styleSheet);
+    const ownerDocument = ownerNode
+      ? ownerNode.ownerDocument
      : this._targetActor.window;

    return getSourcemapBaseURL(
--- a/devtools/server/actors/style-sheet.js
+++ b/devtools/server/actors/style-sheet.js
@ -82,6 +82,34 @@ function getSheetText(sheet) {

 exports.getSheetText = getSheetText;

+/**
+ * For imported stylesheets, `ownerNode` is null.
+ * To resolve the ownerNode for an imported stylesheet, loop on `parentStylesheet`
+ * until we reach the topmost stylesheet, which should have a valid ownerNode.
+ *
+ * @param {StyleSheet}
+ *        The stylesheet for which we want to retrieve the ownerNode.
+ * @return {DOMNode} The ownerNode
+ */
+function getSheetOwnerNode(sheet) {
+  // If this is not an imported stylesheet and we have an ownerNode available
+  // bail out immediately.
+  if (sheet.ownerNode) {
+    return sheet.ownerNode;
+  }
+
+  let parentStyleSheet = sheet;
+  while (
+    parentStyleSheet.parentStyleSheet &&
+    parentStyleSheet !== parentStyleSheet.parentStyleSheet
+  ) {
+    parentStyleSheet = parentStyleSheet.parentStyleSheet;
+  }
+
+  return parentStyleSheet.ownerNode;
+}
+exports.getSheetOwnerNode = getSheetOwnerNode;
+
 /**
 * Get the charset of the stylesheet.
 */
@ -132,10 +160,11 @@ async function fetchStylesheet(sheet) {
  const excludedProtocolsRe = /^(chrome|file|resource|moz-extension):\/\//;
  if (!excludedProtocolsRe.test(href)) {
    // Stylesheets using other protocols should use the content principal.
-    if (sheet.ownerNode) {
+    const ownerNode = getSheetOwnerNode(sheet);
+    if (ownerNode) {
      // eslint-disable-next-line mozilla/use-ownerGlobal
-      options.window = sheet.ownerNode.ownerDocument.defaultView;
-      options.principal = sheet.ownerNode.ownerDocument.nodePrincipal;
+      options.window = ownerNode.ownerDocument.defaultView;
+      options.principal = ownerNode.ownerDocument.nodePrincipal;
    }
  }