Bug 871423 - Open correct inline stylesheet from style inspector link; r=bgrins

browser/devtools/styleeditor/StyleEditorUI.jsm

@@ -500,8 +500,7 @@ StyleEditorUI.prototype = {
           this._selectEditor(editor);
         }
 
-        if (this._styleSheetToSelect
-            && this._styleSheetToSelect.stylesheet == editor.styleSheet.href) {
+        if (this._isEditorToSelect(editor)) {
           this.switchToSelectedSheet();
         }
 
@@ -566,25 +565,42 @@ StyleEditorUI.prototype = {
    *         Promise that will resolve when the editor is selected.
    */
   switchToSelectedSheet: function() {
-    let sheet = this._styleSheetToSelect;
-    let isHref = sheet.stylesheet === null || typeof sheet.stylesheet == "string";
+    let toSelect = this._styleSheetToSelect;
 
     for (let editor of this.editors) {
-      if ((isHref && editor.styleSheet.href == sheet.stylesheet) ||
-          sheet.stylesheet == editor.styleSheet) {
+      if (this._isEditorToSelect(editor)) {
         // The _styleSheetBoundToSelect will always hold the latest pending
         // requested style sheet (with line and column) which is not yet
         // selected by the source editor. Only after we select that particular
         // editor and go the required line and column, it will become null.
         this._styleSheetBoundToSelect = this._styleSheetToSelect;
         this._styleSheetToSelect = null;
-        return this._selectEditor(editor, sheet.line, sheet.col);
+        return this._selectEditor(editor, toSelect.line, toSelect.col);
       }
     }
 
     return promise.resolve();
   },
 
+  /**
+   * Returns whether a given editor is the current editor to be selected. Tests
+   * based on href or underlying stylesheet.
+   *
+   * @param {StyleSheetEditor} editor
+   *        The editor to test.
+   */
+  _isEditorToSelect: function(editor) {
+    let toSelect = this._styleSheetToSelect;
+    if (!toSelect) {
+      return false;
+    }
+    let isHref = toSelect.stylesheet === null ||
+                 typeof toSelect.stylesheet == "string";
+
+    return (isHref && editor.styleSheet.href == toSelect.stylesheet) ||
+           (toSelect.stylesheet == editor.styleSheet);
+  },
+
   /**
    * Select an editor in the UI.
    *

browser/devtools/styleeditor/test/browser_styleeditor_private_perwindowpb.js

@@ -42,6 +42,7 @@ function test() {
 
   function checkCache() {
     checkDiskCacheFor(TEST_HOST, function() {
+      gUI.off("editor-added", onEditorAdded);
       win.close();
       win = null;
       gUI = null;

browser/devtools/styleinspector/computed-view.js

@@ -1424,19 +1424,16 @@ SelectorView.prototype = {
       return;
     }
 
-    let location = promise.resolve({
-      href: rule.href,
-      line: rule.line
-    });
-    if (rule.href && Services.prefs.getBoolPref(PREF_ORIG_SOURCES)) {
+    let location = promise.resolve(rule.location);
+    if (Services.prefs.getBoolPref(PREF_ORIG_SOURCES)) {
       location = rule.getOriginalLocation();
     }
-
-    location.then(({href, line}) => {
+    location.then(({source, href, line, column}) => {
       let target = inspector.target;
       if (ToolDefinitions.styleEditor.isTargetSupported(target)) {
         gDevTools.showToolbox(target, "styleeditor").then(function(toolbox) {
-          toolbox.getCurrentPanel().selectStyleSheet(href, line);
+          let sheet = source || href;
+          toolbox.getCurrentPanel().selectStyleSheet(sheet, line, column);
         });
       }
     });

browser/devtools/styleinspector/style-inspector.js

@@ -61,11 +61,12 @@ function RuleViewTool(aInspector, aWindow, aIFrame)
     if (Services.prefs.getBoolPref(PREF_ORIG_SOURCES)) {
       location = rule.getOriginalLocation();
     }
-    location.then(({ href, line, column }) => {
+    location.then(({ source, href, line, column }) => {
       let target = this.inspector.target;
       if (ToolDefinitions.styleEditor.isTargetSupported(target)) {
         gDevTools.showToolbox(target, "styleeditor").then(function(toolbox) {
-          toolbox.getCurrentPanel().selectStyleSheet(href, line, column);
+          let sheet = source || href;
+          toolbox.getCurrentPanel().selectStyleSheet(sheet, line, column);
         });
       }
       return;

browser/devtools/styleinspector/test/browser_computedview_style-editor-link.js

@@ -21,6 +21,9 @@ const DOCUMENT_URL = "data:text/html;charset=utf-8,"+encodeURIComponent(
    '.nomatches {color: #ff0000;}</style> <div id="first" style="margin: 10em; ',
    'font-size: 14pt; font-family: helvetica, sans-serif; color: #AAA">',
    '</style>',
+   '<style>',
+   'div { color: #f06; }',
+   '</style>',
    '<link rel="stylesheet" type="text/css" href="'+STYLESHEET_URL+'">',
    '</head>',
    '<body>',
@@ -50,7 +53,8 @@ let test = asyncTest(function*() {
   yield selectNode("span", inspector);
 
   yield testInlineStyle(view, inspector);
-  yield testInlineStyleSheet(view, toolbox);
+  yield testFirstInlineStyleSheet(view, toolbox);
+  yield testSecondInlineStyleSheet(view, toolbox);
   yield testExternalStyleSheet(view, toolbox);
 });
 
@@ -61,8 +65,7 @@ function* testInlineStyle(view, inspector) {
 
   let onWindow = waitForWindow();
   info("Clicking on the first rule-link in the computed-view");
-  let link = getComputedViewLinkByIndex(view, 0);
-  link.click();
+  clickLinkByIndex(view, 0);
 
   let win = yield onWindow;
 
@@ -72,15 +75,14 @@ function* testInlineStyle(view, inspector) {
   win.close();
 }
 
-function* testInlineStyleSheet(view, toolbox) {
+function* testFirstInlineStyleSheet(view, toolbox) {
   info("Testing inline stylesheet");
 
   info("Listening for toolbox switch to the styleeditor");
   let onSwitch = waitForStyleEditor(toolbox);
 
   info("Clicking an inline stylesheet");
-  let link = getComputedViewLinkByIndex(view, 2);
-  link.click();
+  clickLinkByIndex(view, 2);
   let editor = yield onSwitch;
 
   ok(true, "Switched to the style-editor panel in the toolbox");
@@ -88,6 +90,24 @@ function* testInlineStyleSheet(view, toolbox) {
   validateStyleEditorSheet(editor, 0);
 }
 
+function* testSecondInlineStyleSheet(view, toolbox) {
+  info("Testing second inline stylesheet");
+
+  info("Waiting for the stylesheet editor to be selected");
+  let panel = toolbox.getCurrentPanel();
+  let onSelected = panel.UI.once("editor-selected");
+
+  info("Switching back to the inspector panel in the toolbox");
+  yield toolbox.selectTool("inspector");
+
+  info("Clicking on second inline stylesheet link");
+  clickLinkByIndex(view, 4);
+  let editor = yield onSelected;
+
+  is(toolbox.currentToolId, "styleeditor", "The style editor is selected again");
+  validateStyleEditorSheet(editor, 1);
+}
+
 function* testExternalStyleSheet(view, toolbox) {
   info("Testing external stylesheet");
 
@@ -99,12 +119,11 @@ function* testExternalStyleSheet(view, toolbox) {
   yield toolbox.selectTool("inspector");
 
   info("Clicking on an external stylesheet link");
-  let link =  getComputedViewLinkByIndex(view, 1);
-  link.click();
+  clickLinkByIndex(view, 1);
   let editor = yield onSelected;
 
   is(toolbox.currentToolId, "styleeditor", "The style editor is selected again");
-  validateStyleEditorSheet(editor, 1);
+  validateStyleEditorSheet(editor, 2);
 }
 
 function validateStyleEditorSheet(editor, expectedSheetIndex) {
@@ -112,3 +131,9 @@ function validateStyleEditorSheet(editor, expectedSheetIndex) {
   let sheet = content.document.styleSheets[expectedSheetIndex];
   is(editor.styleSheet.href, sheet.href, "loaded stylesheet matches document stylesheet");
 }
+
+function clickLinkByIndex(view, index) {
+  let link = getComputedViewLinkByIndex(view, index);
+  link.scrollIntoView();
+  link.click();
+}

browser/devtools/styleinspector/test/browser_ruleview_style-editor-link.js

@@ -24,6 +24,9 @@ const DOCUMENT_URL = "data:text/html;charset=utf-8,"+encodeURIComponent(
    '.nomatches {color: #ff0000;}</style> <div id="first" style="margin: 10em; ',
    'font-size: 14pt; font-family: helvetica, sans-serif; color: #AAA">',
    '</style>',
+   '<style>',
+   'div { font-weight: bold; }',
+   '</style>',
    '<link rel="stylesheet" type="text/css" href="'+STYLESHEET_URL+'">',
    '<link rel="stylesheet" type="text/css" href="'+EXTERNAL_STYLESHEET_URL+'">',
    '</head>',
@@ -52,7 +55,8 @@ let test = asyncTest(function*() {
   yield selectNode("div", inspector);
 
   yield testInlineStyle(view, inspector);
-  yield testInlineStyleSheet(view, toolbox);
+  yield testFirstInlineStyleSheet(view, toolbox);
+  yield testSecondInlineStyleSheet(view, toolbox);
   yield testExternalStyleSheet(view, toolbox);
 });
 
@@ -61,9 +65,7 @@ function* testInlineStyle(view, inspector) {
 
   let onWindow = waitForWindow();
   info("Clicking on the first link in the rule-view");
-  let link = getRuleViewLinkByIndex(view, 0);
-  link.scrollIntoView();
-  link.click();
+  clickLinkByIndex(view, 0);
 
   let win = yield onWindow;
 
@@ -73,16 +75,14 @@ function* testInlineStyle(view, inspector) {
   win.close();
 }
 
-function* testInlineStyleSheet(view, toolbox) {
+function* testFirstInlineStyleSheet(view, toolbox) {
   info("Testing inline stylesheet");
 
   info("Listening for toolbox switch to the styleeditor");
   let onSwitch = waitForStyleEditor(toolbox);
 
   info("Clicking an inline stylesheet");
-  let link = getRuleViewLinkByIndex(view, 4);
-  link.scrollIntoView();
-  link.click();
+  clickLinkByIndex(view, 4);
   let editor = yield onSwitch;
 
   ok(true, "Switched to the style-editor panel in the toolbox");
@@ -90,6 +90,25 @@ function* testInlineStyleSheet(view, toolbox) {
   validateStyleEditorSheet(editor, 0);
 }
 
+function* testSecondInlineStyleSheet(view, toolbox) {
+  info("Testing second inline stylesheet");
+
+  info("Waiting for the stylesheet editor to be selected");
+  let panel = toolbox.getCurrentPanel();
+  let onSelected = panel.UI.once("editor-selected");
+
+  info("Switching back to the inspector panel in the toolbox");
+  yield toolbox.selectTool("inspector");
+
+  info("Clicking on second inline stylesheet link");
+  testRuleViewLinkLabel(view);
+  clickLinkByIndex(view, 3);
+  let editor = yield onSelected;
+
+  is(toolbox.currentToolId, "styleeditor", "The style editor is selected again");
+  validateStyleEditorSheet(editor, 1);
+}
+
 function* testExternalStyleSheet(view, toolbox) {
   info("Testing external stylesheet");
 
@@ -102,19 +121,20 @@ function* testExternalStyleSheet(view, toolbox) {
 
   info("Clicking on an external stylesheet link");
   testRuleViewLinkLabel(view);
-  let link =  getRuleViewLinkByIndex(view, 1);
-  link.scrollIntoView();
-  link.click();
+  clickLinkByIndex(view, 1);
   let editor = yield onSelected;
 
   is(toolbox.currentToolId, "styleeditor", "The style editor is selected again");
-  validateStyleEditorSheet(editor, 1);
+  validateStyleEditorSheet(editor, 2);
 }
 
 function validateStyleEditorSheet(editor, expectedSheetIndex) {
   info("validating style editor stylesheet");
+  is(editor.styleSheet.styleSheetIndex, expectedSheetIndex,
+     "loaded stylesheet index matches document stylesheet");
+
   let sheet = content.document.styleSheets[expectedSheetIndex];
-  is(editor.styleSheet.href, sheet.href, "loaded stylesheet matches document stylesheet");
+  is(editor.styleSheet.href, sheet.href, "loaded stylesheet href matches document stylesheet");
 }
 
 function testRuleViewLinkLabel(view) {
@@ -128,3 +148,9 @@ function testRuleViewLinkLabel(view) {
   is(tooltipText, EXTERNAL_STYLESHEET_URL,
     "rule view stylesheet tooltip text matches the full URI path");
 }
+
+function clickLinkByIndex(view, index) {
+  let link = getRuleViewLinkByIndex(view, index);
+  link.scrollIntoView();
+  link.click();
+}

toolkit/devtools/server/actors/root.js

@@ -13,6 +13,12 @@ const { DebuggerServer } = require("devtools/server/main");
 const { dumpProtocolSpec } = require("devtools/server/protocol");
 const makeDebugger = require("./utils/make-debugger");
 
+Cu.import("resource://gre/modules/XPCOMUtils.jsm");
+
+XPCOMUtils.defineLazyGetter(this, "StyleSheetActor", () => {
+  return require("devtools/server/actors/stylesheets").StyleSheetActor;
+});
+
 /* Root actor for the remote debugging protocol. */
 
 /**
@@ -94,6 +100,9 @@ function RootActor(aConnection, aParameters) {
   this._onAddonListChanged = this.onAddonListChanged.bind(this);
   this._extraActors = {};
 
+  // Map of DOM stylesheets to StyleSheetActors
+  this._styleSheetActors = new Map();
+
   // This creates a Debugger instance for chrome debugging all globals.
   this.makeDebugger = makeDebugger.bind(null, {
     findDebuggees: dbg => dbg.findAllGlobals(),
@@ -226,6 +235,8 @@ RootActor.prototype = {
       this._parameters.onShutdown();
     }
     this._extraActors = null;
+    this._styleSheetActors.clear();
+    this._styleSheetActors = null;
   },
 
   /* The 'listTabs' request and the 'tabListChanged' notification. */
@@ -387,6 +398,28 @@ RootActor.prototype = {
       windowUtils.resumeTimeouts();
       windowUtils.suppressEventHandling(false);
     }
+  },
+
+  /**
+   * Create or return the StyleSheetActor for a style sheet. This method
+   * is here because the Style Editor and Inspector share style sheet actors.
+   *
+   * @param DOMStyleSheet styleSheet
+   *        The style sheet to creat an actor for.
+   * @return StyleSheetActor actor
+   *         The actor for this style sheet.
+   *
+   */
+  createStyleSheetActor: function(styleSheet) {
+    if (this._styleSheetActors.has(styleSheet)) {
+      return this._styleSheetActors.get(styleSheet);
+    }
+    let actor = new StyleSheetActor(styleSheet, this);
+    this._styleSheetActors.set(styleSheet, actor);
+
+    this._globalActorPool.addActor(actor);
+
+    return actor;
   }
 };
 

toolkit/devtools/server/actors/styles.js

@@ -12,7 +12,9 @@ const {Arg, Option, method, RetVal, types} = protocol;
 const events = require("sdk/event/core");
 const object = require("sdk/util/object");
 const { Class } = require("sdk/core/heritage");
-const { StyleSheetActor } = require("devtools/server/actors/stylesheets");
+
+// This will add the "stylesheet" actor type for protocol.js to recognize
+require("devtools/server/actors/stylesheets");
 
 loader.lazyGetter(this, "CssLogic", () => require("devtools/styleinspector/css-logic").CssLogic);
 loader.lazyGetter(this, "DOMUtils", () => Cc["@mozilla.org/inspector/dom-utils;1"].getService(Ci.inIDOMUtils));
@@ -112,17 +114,15 @@ var PageStyleActor = protocol.ActorClass({
   },
 
   /**
-   * Return or create a StyleSheetActor for the given
-   * nsIDOMCSSStyleSheet
+   * Return or create a StyleSheetActor for the given nsIDOMCSSStyleSheet.
+   * @param  {DOMStyleSheet} sheet
+   *         The style sheet to create an actor for.
+   * @return {StyleSheetActor}
+   *         The actor for this style sheet
    */
   _sheetRef: function(sheet) {
-    if (this.refMap.has(sheet)) {
-      return this.refMap.get(sheet);
-    }
-    let actor = new StyleSheetActor(sheet, this, this.walker.rootWin);
-    this.manage(actor);
-    this.refMap.set(sheet, actor);
-
+    let tabActor = this.inspector.tabActor;
+    let actor = tabActor.createStyleSheetActor(sheet);
     return actor;
   },
 
@@ -975,6 +975,7 @@ var StyleRuleFront = protocol.FrontClass(StyleRuleActor, {
   get location()
   {
     return {
+      source: this.parentStyleSheet,
       href: this.href,
       line: this.line,
       column: this.column
@@ -992,12 +993,15 @@ var StyleRuleFront = protocol.FrontClass(StyleRuleActor, {
       return promise.resolve(this.location);
     }
     return parentSheet.getOriginalLocation(this.line, this.column)
-      .then(({ source, line, column }) => {
+      .then(({ fromSourceMap, source, line, column }) => {
         let location = {
           href: source,
           line: line,
           column: column
         }
+        if (fromSourceMap === false) {
+          location.source = this.parentStyleSheet;
+        }
         if (!source) {
           location.href = this.href;
         }

toolkit/devtools/server/actors/stylesheets.js

@@ -74,17 +74,6 @@ let StyleSheetsActor = protocol.ActorClass({
     protocol.Actor.prototype.initialize.call(this, null);
 
     this.parentActor = tabActor;
-
-    // keep a map of sheets-to-actors so we don't create two actors for one sheet
-    this._sheets = new Map();
-  },
-
-  /**
-   * Destroy the current StyleSheetsActor instance.
-   */
-  destroy: function()
-  {
-    this._sheets.clear();
   },
 
   /**
@@ -156,7 +145,7 @@ let StyleSheetsActor = protocol.ActorClass({
     return Task.spawn(function() {
       let actors = [];
       for (let i = 0; i < styleSheets.length; i++) {
-        let actor = this._createStyleSheetActor(styleSheets[i]);
+        let actor = this.parentActor.createStyleSheetActor(styleSheets[i]);
         actors.push(actor);
 
         // Get all sheets, including imported ones
@@ -188,7 +177,7 @@ let StyleSheetsActor = protocol.ActorClass({
           if (!rule.styleSheet) {
             continue;
           }
-          let actor = this._createStyleSheetActor(rule.styleSheet);
+          let actor = this.parentActor.createStyleSheetActor(rule.styleSheet);
           imported.push(actor);
 
           // recurse imports in this stylesheet as well
@@ -205,36 +194,6 @@ let StyleSheetsActor = protocol.ActorClass({
     }.bind(this));
   },
 
-  /**
-   * Create a new actor for a style sheet, if it hasn't already been created.
-   *
-   * @param  {DOMStyleSheet} styleSheet
-   *         The style sheet to create an actor for.
-   * @return {StyleSheetActor}
-   *         The actor for this style sheet
-   */
-  _createStyleSheetActor: function(styleSheet)
-  {
-    if (this._sheets.has(styleSheet)) {
-      return this._sheets.get(styleSheet);
-    }
-    let actor = new StyleSheetActor(styleSheet, this);
-
-    this.manage(actor);
-    this._sheets.set(styleSheet, actor);
-
-    return actor;
-  },
-
-  /**
-   * Clear all the current stylesheet actors in map.
-   */
-  _clearStyleSheetActors: function() {
-    for (let actor in this._sheets) {
-      this.unmanage(this._sheets[actor]);
-    }
-    this._sheets.clear();
-  },
 
   /**
    * Create a new style sheet in the document with the given text.
@@ -255,7 +214,7 @@ let StyleSheetsActor = protocol.ActorClass({
     }
     parent.appendChild(style);
 
-    let actor = this._createStyleSheetActor(style.sheet);
+    let actor = this.parentActor.createStyleSheetActor(style.sheet);
     return actor;
   }, {
     request: { text: Arg(0, "string") },
@@ -323,6 +282,8 @@ let MediaRuleActor = protocol.ActorClass({
     if (this.mql) {
       this.mql.removeListener(this._matchesChange);
     }
+
+    protocol.Actor.prototype.destroy.call(this);
   },
 
   form: function(detail) {
@@ -420,16 +381,6 @@ let StyleSheetActor = protocol.ActorClass({
    */
   get document() this.window.document,
 
-  /**
-   * Browser for the target.
-   */
-  get browser() {
-    if (this.parentActor.parentActor) {
-      return this.parentActor.parentActor.browser;
-    }
-    return null;
-  },
-
   get ownerNode() this.rawSheet.ownerNode,
 
   /**
@@ -768,6 +719,7 @@ let StyleSheetActor = protocol.ActorClass({
         return sourceMap.originalPositionFor({ line: line, column: column });
       }
       return {
+        fromSourceMap: false,
         source: this.href,
         line: line,
         column: column
@@ -998,7 +950,6 @@ var StyleSheetFront = protocol.FrontClass(StyleSheetActor, {
 
   destroy: function() {
     events.off(this, "property-change", this._onPropertyChange);
-
     protocol.Front.prototype.destroy.call(this);
   },
 

toolkit/devtools/server/actors/webbrowser.js

@@ -29,6 +29,10 @@ XPCOMUtils.defineLazyGetter(this, "events", () => {
   return require("sdk/event/core");
 });
 
+XPCOMUtils.defineLazyGetter(this, "StyleSheetActor", () => {
+  return require("devtools/server/actors/stylesheets").StyleSheetActor;
+});
+
 // Also depends on following symbols, shared by common scope with main.js:
 // DebuggerServer, CommonCreateExtraActors, CommonAppendExtraActors, ActorPool,
 // ThreadActor
@@ -545,6 +549,9 @@ function TabActor(aConnection)
   this._extraActors = {};
   this._exited = false;
 
+  // Map of DOM stylesheets to StyleSheetActors
+  this._styleSheetActors = new Map();
+
   this._shouldAddNewGlobalAsDebuggee = this._shouldAddNewGlobalAsDebuggee.bind(this);
 
   this.makeDebugger = makeDebugger.bind(null, {
@@ -722,6 +729,7 @@ TabActor.prototype = {
   disconnect: function BTA_disconnect() {
     this._detach();
     this._extraActors = null;
+    this._styleSheetActors.clear();
     this._exited = true;
   },
 
@@ -1059,6 +1067,12 @@ TabActor.prototype = {
       }
     }
 
+    for (let sheetActor of this._styleSheetActors.values()) {
+      this._tabPool.removeActor(sheetActor);
+    }
+    this._styleSheetActors.clear();
+
+
     // Refresh the debuggee list when a new window object appears (top window or
     // iframe).
     if (threadActor.attached) {
@@ -1175,6 +1189,28 @@ TabActor.prototype = {
     }
     catch (ex) { }
     return isNative;
+  },
+
+  /**
+   * Create or return the StyleSheetActor for a style sheet. This method
+   * is here because the Style Editor and Inspector share style sheet actors.
+   *
+   * @param DOMStyleSheet styleSheet
+   *        The style sheet to creat an actor for.
+   * @return StyleSheetActor actor
+   *         The actor for this style sheet.
+   *
+   */
+  createStyleSheetActor: function BTA_createStyleSheetActor(styleSheet) {
+    if (this._styleSheetActors.has(styleSheet)) {
+      return this._styleSheetActors.get(styleSheet);
+    }
+    let actor = new StyleSheetActor(styleSheet, this);
+    this._styleSheetActors.set(styleSheet, actor);
+
+    this._tabPool.addActor(actor);
+
+    return actor;
   }
 };