Bug 1642371 - Part 4: Open style editor using exact front object where possible. r=jlast

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

devtools/client/framework/test/browser_toolbox_view_source_03.js

@@ -12,10 +12,10 @@ var CSS_URL = `${URL_ROOT}doc_theme.css`;
 async function viewSource() {
   const toolbox = await openNewTabAndToolbox(URL);
 
-  const fileFound = await toolbox.viewSourceInStyleEditor(CSS_URL, 2);
+  const fileFound = await toolbox.viewSourceInStyleEditorByURL(CSS_URL, 2);
   ok(
     fileFound,
-    "viewSourceInStyleEditor should resolve to true if source found."
+    "viewSourceInStyleEditorByURL should resolve to true if source found."
   );
 
   const stylePanel = toolbox.getPanel("styleeditor");

devtools/client/framework/toolbox.js

@@ -3974,16 +3974,83 @@ Toolbox.prototype = {
   },
 
   /**
-   * Opens source in style editor. Falls back to plain "view-source:".
-   * @see devtools/client/shared/source-utils.js
+   * Given a URL for a stylesheet (generated or original), open in the style
+   * editor if possible. Falls back to plain "view-source:".
+   * If the stylesheet has a sourcemap, we will attempt to open the original
+   * version of the file instead of the generated version.
    */
-  viewSourceInStyleEditor: function(sourceURL, sourceLine, sourceColumn) {
-    return viewSource.viewSourceInStyleEditor(
-      this,
-      sourceURL,
-      sourceLine,
-      sourceColumn
-    );
+  viewSourceInStyleEditorByURL: async function(url, line, column) {
+    if (typeof url !== "string") {
+      console.warn("Failed to open source, no url given");
+      return;
+    }
+
+    try {
+      const sourceMappedLoc = await this.sourceMapURLService.originalPositionForURL(
+        url,
+        line,
+        column
+      );
+      if (sourceMappedLoc) {
+        url = sourceMappedLoc.url;
+        line = sourceMappedLoc.line;
+        column = sourceMappedLoc.column;
+      }
+    } catch (err) {
+      console.error(
+        "Failed to resolve sourcemapped location for the given source location",
+        { url, line, column },
+        err
+      );
+    }
+
+    return viewSource.viewSourceInStyleEditor(this, url, line, column);
+  },
+
+  /**
+   * Opens source in style editor. Falls back to plain "view-source:".
+   * If the stylesheet has a sourcemap, we will attempt to open the original
+   * version of the file instead of the generated version.
+   */
+  viewSourceInStyleEditorByFront: async function(
+    stylesheetFront,
+    line,
+    column
+  ) {
+    if (!stylesheetFront || typeof stylesheetFront !== "object") {
+      console.warn("Failed to open source, no stylesheet given");
+      return;
+    }
+
+    let frontOrURL = stylesheetFront;
+    try {
+      const sourceMappedLoc = await this.sourceMapURLService.originalPositionFor(
+        stylesheetFront
+          ? stylesheetFront.href || stylesheetFront.nodeHref
+          : null,
+        line,
+        column
+      );
+      if (sourceMappedLoc) {
+        frontOrURL = sourceMappedLoc.sourceUrl;
+        line = sourceMappedLoc.line;
+        column = sourceMappedLoc.column;
+      }
+    } catch (err) {
+      console.error(
+        "Failed to resolve sourcemapped location for the given source location",
+        {
+          url: stylesheetFront
+            ? stylesheetFront.href || stylesheetFront.nodeHref
+            : null,
+          line,
+          column,
+        },
+        err
+      );
+    }
+
+    return viewSource.viewSourceInStyleEditor(this, frontOrURL, line, column);
   },
 
   viewElementInInspector: async function(objectActor, inspectFromAnnotation) {

devtools/client/inspector/computed/computed.js

@@ -12,7 +12,6 @@ const { ELEMENT_STYLE } = require("devtools/shared/specs/styles");
 const OutputParser = require("devtools/client/shared/output-parser");
 const { PrefObserver } = require("devtools/client/shared/prefs");
 const { createChild } = require("devtools/client/inspector/shared/utils");
-const { gDevTools } = require("devtools/client/framework/devtools");
 const {
   VIEW_NODE_SELECTOR_TYPE,
   VIEW_NODE_PROPERTY_TYPE,
@@ -1355,18 +1354,17 @@ function SelectorView(tree, selectorInfo) {
     const sheet = rule.parentStyleSheet;
     this.source = CssLogic.shortSource(sheet) + ":" + rule.line;
 
-    const url = sheet.href || sheet.nodeHref;
-    this.currentLocation = {
-      href: url,
+    this.generatedLocation = {
+      sheet: sheet,
+      href: sheet.href || sheet.nodeHref,
       line: rule.line,
       column: rule.column,
     };
-    this.generatedLocation = this.currentLocation;
     this.sourceMapURLService = this.tree.inspector.toolbox.sourceMapURLService;
     this.sourceMapURLService.subscribe(
-      url,
-      rule.line,
-      rule.column,
+      this.generatedLocation.href,
+      this.generatedLocation.line,
+      this.generatedLocation.column,
       this._updateLocation
     );
   }
@@ -1481,19 +1479,16 @@ SelectorView.prototype = {
 
     // Update |currentLocation| to be whichever location is being
     // displayed at the moment.
+    let currentLocation = this.generatedLocation;
     if (enabled) {
-      this.currentLocation = { href: url, line, column };
-    } else {
-      this.currentLocation = this.generatedLocation;
+      currentLocation = { href: url, line, column };
     }
 
     const selector = '[sourcelocation="' + this.source + '"]';
     const link = this.tree.element.querySelector(selector);
     if (link) {
       const text =
-        CssLogic.shortSource(this.currentLocation) +
-        ":" +
-        this.currentLocation.line;
+        CssLogic.shortSource(currentLocation) + ":" + currentLocation.line;
       link.textContent = text;
     }
 
@@ -1523,12 +1518,10 @@ SelectorView.prototype = {
       return;
     }
 
-    const { href, line, column } = this.currentLocation;
+    const { sheet, line, column } = this.generatedLocation;
     const target = inspector.currentTarget;
     if (ToolDefinitions.styleEditor.isTargetSupported(target)) {
-      gDevTools.showToolbox(target, "styleeditor").then(function(toolbox) {
-        toolbox.getCurrentPanel().selectStyleSheet(href, line, column);
-      });
+      inspector.toolbox.viewSourceInStyleEditorByFront(sheet, line, column);
     }
   },
 

devtools/client/inspector/markup/markup.js

@@ -1099,7 +1099,7 @@ MarkupView.prototype = {
           if (type === "uri") {
             openContentLink(url);
           } else if (type === "cssresource") {
-            return this.toolbox.viewSourceInStyleEditor(url);
+            return this.toolbox.viewSourceInStyleEditorByURL(url);
           } else if (type === "jsresource") {
             return this.toolbox.viewSourceInDebugger(url);
           }

devtools/client/inspector/rules/models/rule.js

@@ -78,7 +78,7 @@ class Rule {
     this.onDeclarationsUpdated = this.onDeclarationsUpdated.bind(this);
     this.onLocationChanged = this.onLocationChanged.bind(this);
     this.onStyleRuleFrontUpdated = this.onStyleRuleFrontUpdated.bind(this);
-    this.updateSourceLocation = this.updateSourceLocation.bind(this);
+    this.updateOriginalLocation = this.updateOriginalLocation.bind(this);
 
     // Added in Firefox 72 for backwards compatibility of initial fix for Bug 1557689.
     // See follow-up fix in Bug 1593944.
@@ -130,10 +130,8 @@ class Rule {
 
   get sourceLink() {
     return {
-      label: this.getSourceText(
-        CssLogic.shortSource({ href: this.sourceLocation.url })
-      ),
-      title: this.getSourceText(this.sourceLocation.url),
+      label: this._getSourceText(true),
+      title: this._getSourceText(),
     };
   }
 
@@ -145,16 +143,17 @@ class Rule {
    * Returns the original source location which includes the original URL, line and
    * column numbers.
    */
-  get sourceLocation() {
-    if (!this._sourceLocation) {
-      this._sourceLocation = {
-        column: this.ruleColumn,
-        line: this.ruleLine,
+  get generatedLocation() {
+    if (!this._generatedLocation) {
+      this._generatedLocation = {
+        sheet: this.sheet,
         url: this.sheet ? this.sheet.href || this.sheet.nodeHref : null,
+        line: this.ruleLine,
+        column: this.ruleColumn,
       };
     }
 
-    return this._sourceLocation;
+    return this._generatedLocation;
   }
 
   get title() {
@@ -249,23 +248,27 @@ class Rule {
   }
 
   /**
-   * Returns a formatted source text of the given stylesheet URL with its source line
+   * Returns a formatted source text of the stylesheet URL with its source line
    * and @media text.
    *
-   * @param  {String} url
-   *         The stylesheet URL.
+   * @param  {boolean} shortenURL True to get a shorter version of the URL.
    */
-  getSourceText(url) {
+  _getSourceText(shortenURL) {
     if (this.isSystem) {
       return `${STYLE_INSPECTOR_L10N.getStr("rule.userAgentStyles")} ${
         this.title
       }`;
     }
 
-    let sourceText = url;
+    const currentLocation = this._originalLocation || this.generatedLocation;
 
-    if (this.sourceLocation.line > 0) {
-      sourceText += ":" + this.sourceLocation.line;
+    let sourceText = currentLocation.url;
+    if (shortenURL) {
+      sourceText = CssLogic.shortSource({ href: sourceText });
+    }
+
+    if (currentLocation.line > 0) {
+      sourceText += ":" + currentLocation.line;
     }
 
     if (this.mediaText) {
@@ -923,8 +926,13 @@ class Rule {
    * rule. This will overwrite the source map location.
    */
   onLocationChanged() {
-    const url = this.sheet ? this.sheet.href || this.sheet.nodeHref : null;
-    this.updateSourceLocation(url, this.ruleLine, this.ruleColumn);
+    // Clear the cached generated location data so the generatedLocation getter
+    // can rebuild it when needed.
+    this._generatedLocation = null;
+
+    this.store.dispatch(
+      updateSourceLink(this.domRule.actorID, this.sourceLink)
+    );
   }
 
   /**
@@ -932,7 +940,7 @@ class Rule {
    * location.
    */
   subscribeToLocationChange() {
-    const { url, line, column } = this.sourceLocation;
+    const { url, line, column } = this.generatedLocation;
 
     if (url && !this.isSystem && this.domRule.type !== ELEMENT_STYLE) {
       // Subscribe returns an unsubscribe function that can be called on destroy.
@@ -943,7 +951,7 @@ class Rule {
         (enabled, sourceUrl, sourceLine, sourceColumn) => {
           if (enabled) {
             // Only update the source location if source map is in use.
-            this.updateSourceLocation(sourceUrl, sourceLine, sourceColumn);
+            this.updateOriginalLocation(sourceUrl, sourceLine, sourceColumn);
           }
         }
       );
@@ -963,8 +971,8 @@ class Rule {
    * @param  {number} column
    *         The original column number.
    */
-  updateSourceLocation(url, line, column) {
-    this._sourceLocation = {
+  updateOriginalLocation(url, line, column) {
+    this._originalLocation = {
       column,
       line,
       url,

devtools/client/inspector/rules/new-rules.js

@@ -44,12 +44,6 @@ const INSPECTOR_L10N = new LocalizationHelper(
 );
 
 loader.lazyRequireGetter(this, "Tools", "devtools/client/definitions", true);
-loader.lazyRequireGetter(
-  this,
-  "gDevTools",
-  "devtools/client/framework/devtools",
-  true
-);
 loader.lazyRequireGetter(
   this,
   "ClassList",
@@ -470,17 +464,8 @@ class RulesView {
       return;
     }
 
-    const toolbox = await gDevTools.showToolbox(
-      this.currentTarget,
-      "styleeditor"
-    );
-    const styleEditor = toolbox.getCurrentPanel();
-    if (!styleEditor) {
-      return;
-    }
-
-    const { url, line, column } = rule.sourceLocation;
-    styleEditor.selectStyleSheet(url, line, column);
+    const { sheet, line, column } = rule.generatedLocation;
+    this.toolbox.viewSourceInStyleEditorByFront(sheet, line, column);
   }
 
   /**

devtools/client/inspector/rules/test/browser_rules_original-source-link.js

@@ -39,7 +39,7 @@ add_task(async function() {
 
 async function testClickingLink(toolbox, view) {
   info("Listening for switch to the style editor");
-  const onStyleEditorReady = toolbox.once("styleeditor-ready");
+  const onStyleEditorReady = toolbox.once("styleeditor-selected");
 
   info("Finding the stylesheet link and clicking it");
   const link = getRuleViewLinkByIndex(view, 1);
@@ -51,7 +51,7 @@ async function testClickingLink(toolbox, view) {
 function checkDisplayedStylesheet(toolbox) {
   const panel = toolbox.getCurrentPanel();
   return new Promise((resolve, reject) => {
-    panel.UI.on("editor-selected", editor => {
+    const maybeContinue = editor => {
       // The style editor selects the first sheet at first load before
       // selecting the desired sheet.
       if (editor.styleSheet.href.endsWith("scss")) {
@@ -61,7 +61,12 @@ function checkDisplayedStylesheet(toolbox) {
           .then(editorSelected)
           .then(resolve, reject);
       }
-    });
+    };
+    if (panel.UI.selectedEditor) {
+      maybeContinue(panel.UI.selectedEditor);
+    } else {
+      panel.UI.on("editor-selected", maybeContinue);
+    }
   });
 }
 

devtools/client/inspector/rules/test/browser_rules_original-source-link2.js

@@ -39,7 +39,7 @@ add_task(async function() {
 
 async function testClickingLink(toolbox, view) {
   info("Listening for switch to the style editor");
-  const onStyleEditorReady = toolbox.once("styleeditor-ready");
+  const onStyleEditorReady = toolbox.once("styleeditor-selected");
 
   info("Finding the stylesheet link and clicking it");
   const link = getRuleViewLinkByIndex(view, 1);
@@ -51,7 +51,7 @@ async function testClickingLink(toolbox, view) {
 function checkDisplayedStylesheet(toolbox) {
   const panel = toolbox.getCurrentPanel();
   return new Promise((resolve, reject) => {
-    panel.UI.on("editor-selected", editor => {
+    const maybeContinue = editor => {
       // The style editor selects the first sheet at first load before
       // selecting the desired sheet.
       if (editor.styleSheet.href.endsWith("scss")) {
@@ -61,7 +61,12 @@ function checkDisplayedStylesheet(toolbox) {
           .then(editorSelected)
           .then(resolve, reject);
       }
-    });
+    };
+    if (panel.UI.selectedEditor) {
+      maybeContinue(panel.UI.selectedEditor);
+    } else {
+      panel.UI.on("editor-selected", maybeContinue);
+    }
   });
 }
 

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

@@ -32,12 +32,6 @@ const EventEmitter = require("devtools/shared/event-emitter");
 const CssLogic = require("devtools/shared/inspector/css-logic");
 
 loader.lazyRequireGetter(this, "Tools", "devtools/client/definitions", true);
-loader.lazyRequireGetter(
-  this,
-  "gDevTools",
-  "devtools/client/framework/devtools",
-  true
-);
 
 const STYLE_INSPECTOR_PROPERTIES =
   "devtools/shared/locales/styleinspector.properties";
@@ -300,23 +294,18 @@ RuleEditor.prototype = {
   },
 
   _onSourceClick: function() {
-    if (this.source.hasAttribute("unselectable") || !this._currentLocation) {
+    if (this.source.hasAttribute("unselectable")) {
       return;
     }
 
-    const target = this.ruleView.inspector.currentTarget;
+    const { inspector } = this.ruleView;
+    const target = inspector.currentTarget;
     if (Tools.styleEditor.isTargetSupported(target)) {
-      gDevTools.showToolbox(target, "styleeditor").then(toolbox => {
-        const { url, line, column } = this._currentLocation;
-
-        if (!this.rule.sheet.href && this.rule.sheet.nodeHref) {
-          toolbox
-            .getCurrentPanel()
-            .selectStyleSheet(this.rule.sheet, line, column);
-        } else {
-          toolbox.getCurrentPanel().selectStyleSheet(url, line, column);
-        }
-      });
+      inspector.toolbox.viewSourceInStyleEditorByFront(
+        this.rule.sheet,
+        this.rule.ruleLine,
+        this.rule.ruleColumn
+      );
     }
   },
 
@@ -334,28 +323,17 @@ RuleEditor.prototype = {
    *        The original URL
    * @param {Number} line
    *        The original line number
-   * @param {number} column
-   *        The original column number
    */
-  _updateLocation: function(enabled, url, line, column) {
+  _updateLocation: function(enabled, url, line) {
     let displayURL = url;
     if (!enabled) {
-      url = null;
       displayURL = null;
       if (this.rule.sheet) {
-        url = this.rule.sheet.href || this.rule.sheet.nodeHref;
         displayURL = this.rule.sheet.href;
       }
       line = this.rule.ruleLine;
-      column = this.rule.ruleColumn;
     }
 
-    this._currentLocation = {
-      url,
-      line,
-      column,
-    };
-
     let sourceTextContent = CssLogic.shortSource({ href: displayURL });
     let title = displayURL ? displayURL : sourceTextContent;
     if (line > 0) {

devtools/client/locales/en-US/toolbox.properties

@@ -59,7 +59,7 @@ toolbox.noContentProcessForTab.message=No content process for this tab.
 # LOCALIZATION NOTE (toolbox.viewCssSourceInStyleEditor.label)
 # Used as a message in either tooltips or contextual menu items to open the
 # corresponding URL as a css file in the Style-Editor tool.
-# DEV NOTE: Mostly used wherever toolbox.viewSourceInStyleEditor is used.
+# DEV NOTE: Mostly used wherever toolbox.viewSourceInStyleEditorByXX is used.
 toolbox.viewCssSourceInStyleEditor.label=Open File in Style-Editor
 
 # LOCALIZATION NOTE (toolbox.viewJsSourceInDebugger.label)

devtools/client/netmonitor/src/widgets/RequestListContextMenu.js

@@ -442,7 +442,7 @@ class RequestListContextMenu {
    */
   openInStyleEditor(url) {
     const toolbox = gDevTools.getToolbox(this.props.connector.getTabTarget());
-    toolbox.viewSourceInStyleEditor(url, 0);
+    toolbox.viewSourceInStyleEditorByURL(url, 0);
   }
 
   /**

devtools/client/styleeditor/panel.js

@@ -95,8 +95,8 @@ StyleEditorPanel.prototype = {
   /**
    * Select a stylesheet.
    *
-   * @param {string} href
-   *        Url of stylesheet to find and select in editor
+   * @param {string|StyleSheetFront} href
+   *        Url or front of stylesheet to find and select in editor.
    * @param {number} line
    *        Line number to jump to after selecting. One-indexed
    * @param {number} col
@@ -109,6 +109,7 @@ StyleEditorPanel.prototype = {
     if (!this.UI) {
       return null;
     }
+
     return this.UI.selectStyleSheet(href, line - 1, col ? col - 1 : 0);
   },
 

devtools/client/styleeditor/test/browser_styleeditor_selectstylesheet.js

@@ -15,7 +15,7 @@ add_task(async function() {
   const editor = ui.editors[1];
 
   info("Selecting style sheet #1.");
-  await ui.selectStyleSheet(editor.styleSheet.href, LINE_NO);
+  await ui.selectStyleSheet(editor.styleSheet, LINE_NO);
 
   is(ui.selectedEditor, ui.editors[1], "Second editor is selected.");
   const { line, ch } = ui.selectedEditor.sourceEditor.getCursor();

devtools/client/webconsole/webconsole.js

@@ -223,27 +223,6 @@ class WebConsole {
     });
   }
 
-  /**
-   * Tries to open a Stylesheet file related to the web page for the web console
-   * instance in the Style Editor. If the file is not found, it is opened in
-   * source view instead.
-   *
-   * Manually handle the case where toolbox does not exist (Browser Console).
-   *
-   * @param string sourceURL
-   *        The URL of the file.
-   * @param integer sourceLine
-   *        The line number which you want to place the caret.
-   */
-  viewSourceInStyleEditor(sourceURL, sourceLine) {
-    const { toolbox } = this;
-    if (!toolbox) {
-      this.viewSource(sourceURL, sourceLine);
-      return;
-    }
-    toolbox.viewSourceInStyleEditor(sourceURL, sourceLine);
-  }
-
   /**
    * Tries to open a JavaScript file related to the web page for the web console
    * instance in the Script Debugger. If the file is not found, it is opened in
@@ -396,7 +375,7 @@ class WebConsole {
     if (!this.toolbox) {
       return;
     }
-    await this.toolbox.viewSourceInStyleEditor(
+    await this.toolbox.viewSourceInStyleEditorByURL(
       frame.url,
       frame.line,
       frame.column