Bug 1834725 - [devtools] Tweak original CSS handling in the Style Editor. r=devtools-reviewers,nchevobbe

* Accept exceptions from `getOriginalURLS` (see next changeset).
* Avoid instantiating an editor for minified stylesheet when we have a functional original stylesheet.
  We were doing that solely for getting their `friendlyName`, whereas we could lazily compute it only when we need it.

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

devtools/client/inspector/rules/test/browser_rules_style-editor-link.js

@@ -210,7 +210,7 @@ async function testRuleViewLinkLabel(view) {
 
   is(
     value,
-    encodeURIComponent(STYLESHEET_DATA_URL_CONTENTS) + ":1",
+    STYLESHEET_DATA_URL_CONTENTS + ":1",
     "Rule view data URL stylesheet display value matches contents"
   );
   is(

devtools/client/styleeditor/StyleEditorUI.sys.mjs

@@ -20,6 +20,9 @@ import { StyleSheetEditor } from "resource://devtools/client/styleeditor/StyleSh
 const { PrefObserver } = require("resource://devtools/client/shared/prefs.js");
 
 const KeyShortcuts = require("resource://devtools/client/shared/key-shortcuts.js");
+const {
+  shortSource,
+} = require("resource://devtools/shared/inspector/css-logic.js");
 
 const lazy = {};
 
@@ -562,62 +565,87 @@ export class StyleEditorUI extends EventEmitter {
   #addStyleSheet(resource) {
     if (!this.#seenSheets.has(resource)) {
       const promise = (async () => {
-        let editor = await this.#addStyleSheetEditor(resource);
-
-        const sourceMapLoader = this.#toolbox.sourceMapLoader;
-
-        if (
-          !sourceMapLoader ||
-          !Services.prefs.getBoolPref(PREF_ORIG_SOURCES)
-        ) {
-          return editor;
+        // When the StyleSheet is mapped to one or many original sources,
+        // do not create an editor for the minified StyleSheet.
+        const hasValidOriginalSource = await this.#tryAddingOriginalStyleSheets(
+          resource
+        );
+        if (hasValidOriginalSource) {
+          return null;
         }
-
-        const {
-          href,
-          nodeHref,
-          resourceId: id,
-          sourceMapURL,
-          sourceMapBaseURL,
-        } = resource;
-        const sources = await sourceMapLoader.getOriginalURLs({
-          id,
-          url: href || nodeHref,
-          sourceMapBaseURL,
-          sourceMapURL,
-        });
-        // A single generated sheet might map to multiple original
-        // sheets, so make editors for each of them.
-        if (sources && sources.length) {
-          const parentEditorName = editor.friendlyName;
-          this.#removeStyleSheetEditor(editor);
-          editor = null;
-
-          for (const { id: originalId, url: originalURL } of sources) {
-            const original = new lazy.OriginalSource(
-              originalURL,
-              originalId,
-              sourceMapLoader
-            );
-
-            // set so the first sheet will be selected, even if it's a source
-            original.styleSheetIndex = resource.styleSheetIndex;
-            original.relatedStyleSheet = resource;
-            original.relatedEditorName = parentEditorName;
-            original.resourceId = resource.resourceId;
-            original.targetFront = resource.targetFront;
-            original.atRules = resource.atRules;
-            await this.#addStyleSheetEditor(original);
-          }
-        }
-
-        return editor;
+        // Otherwise, if source-map failed or this is a non-source-map CSS
+        // create an editor for it.
+        return this.#addStyleSheetEditor(resource);
       })();
       this.#seenSheets.set(resource, promise);
     }
     return this.#seenSheets.get(resource);
   }
 
+  /**
+   * Check if the given StyleSheet relates to an original StyleSheet (via source maps).
+   * If one is found, create an editor for the original one.
+   *
+   * @param  {Resource} resource
+   *         The STYLESHEET resource which is received from resource command.
+   * @return Boolean
+   *         Return true, when we found a viable related original StyleSheet.
+   */
+  async #tryAddingOriginalStyleSheets(resource) {
+    // Avoid querying the SourceMap if this feature is disabled.
+    if (!Services.prefs.getBoolPref(PREF_ORIG_SOURCES)) {
+      return false;
+    }
+
+    const sourceMapLoader = this.#toolbox.sourceMapLoader;
+    const {
+      href,
+      nodeHref,
+      resourceId: id,
+      sourceMapURL,
+      sourceMapBaseURL,
+    } = resource;
+    let sources;
+    try {
+      sources = await sourceMapLoader.getOriginalURLs({
+        id,
+        url: href || nodeHref,
+        sourceMapBaseURL,
+        sourceMapURL,
+      });
+    } catch (e) {
+      // Ignore any source map error, they will be logged
+      // via the SourceMapLoader and Toolbox into the Web Console.
+      return false;
+    }
+
+    // Return the generated CSS if the source-map failed to be parsed
+    // or did not generate any original source.
+    if (!sources || !sources.length) {
+      return false;
+    }
+
+    // A single generated sheet might map to multiple original
+    // sheets, so make editors for each of them.
+    for (const { id: originalId, url: originalURL } of sources) {
+      const original = new lazy.OriginalSource(
+        originalURL,
+        originalId,
+        sourceMapLoader
+      );
+
+      // set so the first sheet will be selected, even if it's a source
+      original.styleSheetIndex = resource.styleSheetIndex;
+      original.relatedStyleSheet = resource;
+      original.resourceId = resource.resourceId;
+      original.targetFront = resource.targetFront;
+      original.atRules = resource.atRules;
+      await this.#addStyleSheetEditor(original);
+    }
+
+    return true;
+  }
+
   #removeStyleSheet(resource, editor) {
     this.#seenSheets.delete(resource);
     this.#removeStyleSheetEditor(editor);
@@ -1273,8 +1301,13 @@ export class StyleEditorUI extends EventEmitter {
     let linkedCSSSource = "";
     if (editor.linkedCSSFile) {
       linkedCSSSource = PathUtils.filename(editor.linkedCSSFile);
-    } else if (editor.styleSheet.relatedEditorName) {
-      linkedCSSSource = editor.styleSheet.relatedEditorName;
+    } else if (editor.styleSheet.relatedStyleSheet) {
+      // Compute a friendly name for the related generated source
+      // (relatedStyleSheet is set on original CSS to refer to the generated one)
+      linkedCSSSource = shortSource(editor.styleSheet.relatedStyleSheet);
+      try {
+        linkedCSSSource = decodeURI(linkedCSSSource);
+      } catch (e) {}
     }
     text(summary, ".stylesheet-linked-file", linkedCSSSource);
     text(summary, ".stylesheet-title", editor.styleSheet.title || "");

devtools/client/styleeditor/test/browser_styleeditor_enabled.js

@@ -105,6 +105,12 @@ add_task(async function testSystemStylesheet() {
   const formsToggle = formsEditor.summary.querySelector(".stylesheet-toggle");
   ok(formsToggle, "enabled toggle button exists");
   ok(formsToggle.disabled, "enabled toggle button is disabled");
+  // For some unexplained reason, this is updated asynchronously
+  await waitFor(
+    () =>
+      formsToggle.getAttribute("tooltiptext") ==
+      "System style sheets can’t be disabled"
+  );
   is(
     formsToggle.getAttribute("tooltiptext"),
     "System style sheets can’t be disabled"

devtools/shared/inspector/css-logic.js

@@ -153,35 +153,43 @@ exports.shortSource = function (sheet) {
     );
   }
 
+  let name = sheet.href;
+
   // If the sheet is a data URL, return a trimmed version of it.
   const dataUrl = sheet.href.trim().match(/^data:.*?,((?:.|\r|\n)*)$/);
   if (dataUrl) {
-    return dataUrl[1].length > MAX_DATA_URL_LENGTH
-      ? `${dataUrl[1].substr(0, MAX_DATA_URL_LENGTH - 1)}…`
-      : dataUrl[1];
-  }
-
-  // We try, in turn, the filename, filePath, query string, whole thing
-  let url = {};
-  try {
-    url = new URL(sheet.href);
-  } catch (ex) {
-    // Some UA-provided stylesheets are not valid URLs.
-  }
-
-  if (url.pathname) {
-    const index = url.pathname.lastIndexOf("/");
-    if (index !== -1 && index < url.pathname.length) {
-      return url.pathname.slice(index + 1);
+    name =
+      dataUrl[1].length > MAX_DATA_URL_LENGTH
+        ? `${dataUrl[1].substr(0, MAX_DATA_URL_LENGTH - 1)}…`
+        : dataUrl[1];
+  } else {
+    // We try, in turn, the filename, filePath, query string, whole thing
+    let url = {};
+    try {
+      url = new URL(sheet.href);
+    } catch (ex) {
+      // Some UA-provided stylesheets are not valid URLs.
+    }
+
+    if (url.pathname) {
+      const index = url.pathname.lastIndexOf("/");
+      if (index !== -1 && index < url.pathname.length) {
+        name = url.pathname.slice(index + 1);
+      } else {
+        name = url.pathname;
+      }
+    } else if (url.query) {
+      name = url.query;
     }
-    return url.pathname;
   }
 
-  if (url.query) {
-    return url.query;
+  try {
+    name = decodeURIComponent(name);
+  } catch (e) {
+    // This may still fail if the URL contains invalid % numbers (for ex)
   }
 
-  return sheet.href;
+  return name;
 };
 
 /**