Bug 1523128 - Implement open source link in the new rules view. r=rcaliman

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

devtools/client/inspector/rules/actions/index.js

@@ -34,4 +34,10 @@ createEnum([
   // Updates the rules state with the new list of CSS rules for the selected element.
   "UPDATE_RULES",
 
+  // Updates whether or not the source links are enabled.
+  "UPDATE_SOURCE_LINK_ENABLED",
+
+  // Updates the source link information for a given rule.
+  "UPDATE_SOURCE_LINK",
+
 ], module.exports);

devtools/client/inspector/rules/actions/rules.js

@@ -8,6 +8,8 @@ const {
   UPDATE_ADD_RULE_ENABLED,
   UPDATE_HIGHLIGHTED_SELECTOR,
   UPDATE_RULES,
+  UPDATE_SOURCE_LINK_ENABLED,
+  UPDATE_SOURCE_LINK,
 } = require("./index");
 
 module.exports = {
@@ -51,4 +53,33 @@ module.exports = {
     };
   },
 
+  /**
+   * Updates whether or not the source links are enabled.
+   *
+   * @param  {Boolean} enabled
+   *         Whether or not the source links are enabled.
+   */
+  updateSourceLinkEnabled(enabled) {
+    return {
+      type: UPDATE_SOURCE_LINK_ENABLED,
+      enabled,
+    };
+  },
+
+  /**
+   * Updates the source link information for a given rule.
+   *
+   * @param  {String} ruleId
+   *         The Rule id of the target rule.
+   * @param  {Object} sourceLink
+   *         New source link data.
+   */
+  updateSourceLink(ruleId, sourceLink) {
+    return {
+      type: UPDATE_SOURCE_LINK,
+      ruleId,
+      sourceLink,
+    };
+  },
+
 };

devtools/client/inspector/rules/components/Rule.js

@@ -19,6 +19,7 @@ const Types = require("../types");
 class Rule extends PureComponent {
   static get propTypes() {
     return {
+      onOpenSourceLink: PropTypes.func.isRequired,
       onToggleDeclaration: PropTypes.func.isRequired,
       onToggleSelectorHighlighter: PropTypes.func.isRequired,
       rule: PropTypes.shape(Types.rule).isRequired,
@@ -63,6 +64,7 @@ class Rule extends PureComponent {
 
   render() {
     const {
+      onOpenSourceLink,
       onToggleDeclaration,
       onToggleSelectorHighlighter,
       rule,
@@ -87,7 +89,13 @@ class Rule extends PureComponent {
                      (isUnmatched ? " unmatched" : "") +
                      (isUserAgentStyle ? " uneditable" : ""),
         },
-        SourceLink({ sourceLink }),
+        SourceLink({
+          id,
+          isUserAgentStyle,
+          onOpenSourceLink,
+          sourceLink,
+          type,
+        }),
         dom.div({ className: "ruleview-code" },
           dom.div({},
             Selector({

devtools/client/inspector/rules/components/Rules.js

@@ -14,6 +14,7 @@ const Types = require("../types");
 class Rules extends PureComponent {
   static get propTypes() {
     return {
+      onOpenSourceLink: PropTypes.func.isRequired,
       onToggleDeclaration: PropTypes.func.isRequired,
       onToggleSelectorHighlighter: PropTypes.func.isRequired,
       rules: PropTypes.arrayOf(PropTypes.shape(Types.rule)).isRequired,
@@ -26,6 +27,7 @@ class Rules extends PureComponent {
 
   render() {
     const {
+      onOpenSourceLink,
       onToggleDeclaration,
       onToggleSelectorHighlighter,
       rules,
@@ -38,6 +40,7 @@ class Rules extends PureComponent {
     return rules.map(rule => {
       return Rule({
         key: rule.id,
+        onOpenSourceLink,
         onToggleDeclaration,
         onToggleSelectorHighlighter,
         rule,

devtools/client/inspector/rules/components/RulesApp.js

@@ -30,6 +30,7 @@ class RulesApp extends PureComponent {
     return {
       onAddClass: PropTypes.func.isRequired,
       onAddRule: PropTypes.func.isRequired,
+      onOpenSourceLink: PropTypes.func.isRequired,
       onSetClassState: PropTypes.func.isRequired,
       onToggleClassPanelExpanded: PropTypes.func.isRequired,
       onToggleDeclaration: PropTypes.func.isRequired,
@@ -45,6 +46,7 @@ class RulesApp extends PureComponent {
 
   getRuleProps() {
     return {
+      onOpenSourceLink: this.props.onOpenSourceLink,
       onToggleDeclaration: this.props.onToggleDeclaration,
       onToggleSelectorHighlighter: this.props.onToggleSelectorHighlighter,
       showDeclarationNameEditor: this.props.showDeclarationNameEditor,

devtools/client/inspector/rules/components/SourceLink.js

@@ -7,27 +7,72 @@
 const { PureComponent } = require("devtools/client/shared/vendor/react");
 const dom = require("devtools/client/shared/vendor/react-dom-factories");
 const PropTypes = require("devtools/client/shared/vendor/react-prop-types");
+const { connect } = require("devtools/client/shared/vendor/react-redux");
+const { ELEMENT_STYLE } = require("devtools/client/inspector/rules/constants");
 
 const Types = require("../types");
 
 class SourceLink extends PureComponent {
   static get propTypes() {
     return {
+      id: PropTypes.string.isRequired,
+      isSourceLinkEnabled: PropTypes.bool.isRequired,
+      isUserAgentStyle: PropTypes.bool.isRequired,
+      onOpenSourceLink: PropTypes.func.isRequired,
       sourceLink: PropTypes.shape(Types.sourceLink).isRequired,
+      type: PropTypes.number.isRequired,
     };
   }
 
+  constructor(props) {
+    super(props);
+    this.onSourceClick = this.onSourceClick.bind(this);
+  }
+
+  /**
+   * Whether or not the source link is enabled. The source link is enabled when the
+   * Style Editor is enabled and the rule is not a user agent or element style.
+   */
+  get isSourceLinkEnabled() {
+    return this.props.isSourceLinkEnabled &&
+           !this.props.isUserAgentStyle &&
+           this.props.type !== ELEMENT_STYLE;
+  }
+
+  onSourceClick(event) {
+    event.stopPropagation();
+
+    if (this.isSourceLinkEnabled) {
+      this.props.onOpenSourceLink(this.props.id);
+    }
+  }
+
   render() {
-    const { sourceLink } = this.props;
+    const { label, title } = this.props.sourceLink;
 
     return (
-      dom.div({ className: "ruleview-rule-source theme-link" },
-        dom.span({ className: "ruleview-rule-source-label" },
-          sourceLink.title
+      dom.div(
+        {
+          className: "ruleview-rule-source theme-link" +
+                     (!this.isSourceLinkEnabled ? " disabled" : ""),
+          onClick: this.onSourceClick,
+        },
+        dom.span(
+          {
+            className: "ruleview-rule-source-label",
+            title,
+          },
+          label
         )
       )
     );
   }
 }
 
-module.exports = SourceLink;
+const mapStateToProps = state => {
+  return {
+    isSourceLinkEnabled: state.rules.isSourceLinkEnabled,
+  };
+};
+
+module.exports = connect(mapStateToProps)(SourceLink);

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

@@ -74,6 +74,8 @@ class ElementStyle {
       if (rule.editor) {
         rule.editor.destroy();
       }
+
+      rule.destroy();
     }
 
     if (this.ruleView.isNewRulesView) {
@@ -123,6 +125,10 @@ class ElementStyle {
 
       this._sortRulesForPseudoElement();
 
+      if (this.ruleView.isNewRulesView) {
+        this.subscribeRulesToLocationChange();
+      }
+
       // We're done with the previous list of rules.
       for (const r of existingRules) {
         if (r && r.editor) {
@@ -349,10 +355,10 @@ class ElementStyle {
   /**
    * Adds a new declaration to the rule.
    *
-   * @param {String} ruleId
-   *        The id of the Rule to be modified.
-   * @param {String} value
-   *        The new declaration value.
+   * @param  {String} ruleId
+   *         The id of the Rule to be modified.
+   * @param  {String} value
+   *         The new declaration value.
    */
   addNewDeclaration(ruleId, value) {
     const rule = this.getRule(ruleId);
@@ -525,10 +531,10 @@ class ElementStyle {
   /**
    * Modifies the existing rule's selector to the new given value.
    *
-   * @param {String} ruleId
-   *        The id of the Rule to be modified.
-   * @param {String} selector
-   *        The new selector value.
+   * @param  {String} ruleId
+   *         The id of the Rule to be modified.
+   * @param  {String} selector
+   *         The new selector value.
    */
   async modifySelector(ruleId, selector) {
     try {
@@ -589,13 +595,22 @@ class ElementStyle {
     }
   }
 
+  /**
+   * Subscribes all the rules to location changes.
+   */
+  subscribeRulesToLocationChange() {
+    for (const rule of this.rules) {
+      rule.subscribeToLocationChange();
+    }
+  }
+
   /**
    * Toggles the enabled state of the given CSS declaration.
    *
-   * @param {String} ruleId
-   *        The Rule id of the given CSS declaration.
-   * @param {String} declarationId
-   *        The TextProperty id for the CSS declaration.
+   * @param  {String} ruleId
+   *         The Rule id of the given CSS declaration.
+   * @param  {String} declarationId
+   *         The TextProperty id for the CSS declaration.
    */
   toggleDeclaration(ruleId, declarationId) {
     const rule = this.getRule(ruleId);

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

@@ -12,6 +12,7 @@ const {ELEMENT_STYLE} = require("devtools/shared/specs/styles");
 const TextProperty = require("devtools/client/inspector/rules/models/text-property");
 const Services = require("Services");
 
+loader.lazyRequireGetter(this, "updateSourceLink", "devtools/client/inspector/rules/actions/rules", true);
 loader.lazyRequireGetter(this, "promiseWarn", "devtools/client/inspector/shared/utils", true);
 loader.lazyRequireGetter(this, "parseNamedDeclarations", "devtools/shared/css/parsing-utils", true);
 
@@ -50,6 +51,8 @@ class Rule {
 
     this.mediaText = this.domRule && this.domRule.mediaText ? this.domRule.mediaText : "";
     this.cssProperties = this.elementStyle.ruleView.cssProperties;
+    this.inspector = this.elementStyle.ruleView.inspector;
+    this.store = this.elementStyle.ruleView.store;
 
     // Populate the text properties with the style's current authoredText
     // value, and add in any disabled properties from the store.
@@ -57,6 +60,16 @@ class Rule {
     this.textProps = this.textProps.concat(this._getDisabledProperties());
 
     this.getUniqueSelector = this.getUniqueSelector.bind(this);
+    this.onLocationChanged = this.onLocationChanged.bind(this);
+    this.updateSourceLocation = this.updateSourceLocation.bind(this);
+  }
+
+  destroy() {
+    if (this.unsubscribeSourceMap) {
+      this.unsubscribeSourceMap();
+    }
+
+    this.domRule.off("location-changed", this.onLocationChanged);
   }
 
   get declarations() {
@@ -85,13 +98,31 @@ class Rule {
 
   get sourceLink() {
     return {
-      column: this.ruleColumn,
-      line: this.ruleLine,
-      mediaText: this.mediaText,
-      title: this.title,
+      label: this.getSourceText(CssLogic.shortSource({ href: this.sourceLocation.url })),
+      title: this.getSourceText(this.sourceLocation.url),
     };
   }
 
+  get sourceMapURLService() {
+    return this.inspector.toolbox.sourceMapURLService;
+  }
+
+  /**
+   * 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,
+        url: this.sheet ? this.sheet.href || this.sheet.nodeHref : null,
+      };
+    }
+
+    return this._sourceLocation;
+  }
+
   get title() {
     let title = CssLogic.shortSource(this.sheet);
     if (this.domRule.type !== ELEMENT_STYLE && this.ruleLine > 0) {
@@ -178,6 +209,31 @@ class Rule {
     return this.textProps.find(textProp => textProp.id === id);
   }
 
+  /**
+   * Returns a formatted source text of the given stylesheet URL with its source line
+   * and @media text.
+   *
+   * @param  {String} url
+   *         The stylesheet URL.
+   */
+  getSourceText(url) {
+    if (this.isSystem) {
+      return `${STYLE_INSPECTOR_L10N.getStr("rule.userAgentStyles")} ${this.title}`;
+    }
+
+    let sourceText = url;
+
+    if (this.sourceLocation.line > 0) {
+      sourceText += ":" + this.sourceLocation.line;
+    }
+
+    if (this.mediaText) {
+      sourceText += " @media " + this.mediaText;
+    }
+
+    return sourceText;
+  }
+
   /**
    * Returns an unique selector for the CSS rule.
    */
@@ -193,7 +249,7 @@ class Rule {
       selector = await this.inherited.getUniqueSelector();
     } else {
       // This is an inline style from the current node.
-      selector = this.elementStyle.ruleView.inspector.selectionCssSelector;
+      selector = this.inspector.selectionCssSelector;
     }
 
     return selector;
@@ -744,6 +800,57 @@ class Rule {
     }
     return false;
   }
+
+  /**
+   * Handler for "location-changed" events fired from the StyleRuleActor. This could
+   * occur by adding a new declaration to the rule. Updates the source location of the
+   * 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);
+  }
+
+  /**
+   * Subscribes the rule to the source map service to map the the original source
+   * location.
+   */
+  subscribeToLocationChange() {
+    const { url, line, column } = this.sourceLocation;
+
+    if (url && !this.isSystem && this.domRule.type !== ELEMENT_STYLE) {
+      // Subscribe returns an unsubscribe function that can be called on destroy.
+      this.unsubscribeSourceMap = this.sourceMapURLService.subscribe(url, line, column,
+        (enabled, sourceUrl, sourceLine, sourceColumn) => {
+          if (enabled) {
+            // Only update the source location if source map is in use.
+            this.updateSourceLocation(sourceUrl, sourceLine, sourceColumn);
+          }
+        });
+    }
+
+    this.domRule.on("location-changed", this.onLocationChanged);
+  }
+
+  /**
+   * Handler for any location changes called from the SourceMapURLService and can also be
+   * called from onLocationChanged(). Updates the source location for the rule.
+   *
+   * @param  {String} url
+   *         The original URL.
+   * @param  {Number} line
+   *         The original line number.
+   * @param  {number} column
+   *         The original column number.
+   */
+  updateSourceLocation(url, line, column) {
+    this._sourceLocation = {
+      column,
+      line,
+      url,
+    };
+    this.store.dispatch(updateSourceLink(this.domRule.actorID, this.sourceLink));
+  }
 }
 
 module.exports = Rule;

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

@@ -23,6 +23,7 @@ const {
   updateAddRuleEnabled,
   updateHighlightedSelector,
   updateRules,
+  updateSourceLinkEnabled,
 } = require("./actions/rules");
 
 const RulesApp = createFactory(require("./components/RulesApp"));
@@ -31,6 +32,8 @@ const { LocalizationHelper } = require("devtools/shared/l10n");
 const INSPECTOR_L10N =
   new LocalizationHelper("devtools/client/locales/inspector.properties");
 
+loader.lazyRequireGetter(this, "Tools", "devtools/client/definitions", true);
+loader.lazyRequireGetter(this, "gDevTools", "devtools/client/framework/devtools", true);
 loader.lazyRequireGetter(this, "ClassList", "devtools/client/inspector/rules/models/class-list");
 loader.lazyRequireGetter(this, "advanceValidate", "devtools/client/inspector/shared/utils", true);
 loader.lazyRequireGetter(this, "AutocompletePopup", "devtools/client/shared/autocomplete-popup");
@@ -54,11 +57,13 @@ class RulesView {
 
     this.onAddClass = this.onAddClass.bind(this);
     this.onAddRule = this.onAddRule.bind(this);
+    this.onOpenSourceLink = this.onOpenSourceLink.bind(this);
     this.onSelection = this.onSelection.bind(this);
     this.onSetClassState = this.onSetClassState.bind(this);
     this.onToggleClassPanelExpanded = this.onToggleClassPanelExpanded.bind(this);
     this.onToggleDeclaration = this.onToggleDeclaration.bind(this);
     this.onTogglePseudoClass = this.onTogglePseudoClass.bind(this);
+    this.onToolChanged = this.onToolChanged.bind(this);
     this.onToggleSelectorHighlighter = this.onToggleSelectorHighlighter.bind(this);
     this.showDeclarationNameEditor = this.showDeclarationNameEditor.bind(this);
     this.showDeclarationValueEditor = this.showDeclarationValueEditor.bind(this);
@@ -70,6 +75,8 @@ class RulesView {
     this.inspector.sidebar.on("select", this.onSelection);
     this.selection.on("detached-front", this.onSelection);
     this.selection.on("new-node-front", this.onSelection);
+    this.toolbox.on("tool-registered", this.onToolChanged);
+    this.toolbox.on("tool-unregistered", this.onToolChanged);
 
     this.init();
 
@@ -84,6 +91,7 @@ class RulesView {
     const rulesApp = RulesApp({
       onAddClass: this.onAddClass,
       onAddRule: this.onAddRule,
+      onOpenSourceLink: this.onOpenSourceLink,
       onSetClassState: this.onSetClassState,
       onToggleClassPanelExpanded: this.onToggleClassPanelExpanded,
       onToggleDeclaration: this.onToggleDeclaration,
@@ -110,6 +118,8 @@ class RulesView {
     this.inspector.sidebar.off("select", this.onSelection);
     this.selection.off("detached-front", this.onSelection);
     this.selection.off("new-node-front", this.onSelection);
+    this.toolbox.off("tool-registered", this.onToolChanged);
+    this.toolbox.off("tool-unregistered", this.onToolChanged);
 
     if (this._autocompletePopup) {
       this._autocompletePopup.destroy();
@@ -282,6 +292,28 @@ class RulesView {
     await this.elementStyle.addNewRule();
   }
 
+  /**
+   * Handler for opening the source link of the given rule in the Style Editor.
+   *
+   * @param  {String} ruleId
+   *         The id of the Rule for opening the source link.
+   */
+  async onOpenSourceLink(ruleId) {
+    const rule = this.elementStyle.getRule(ruleId);
+    if (!rule || !Tools.styleEditor.isTargetSupported(this.inspector.target)) {
+      return;
+    }
+
+    const toolbox = await gDevTools.showToolbox(this.inspector.target, "styleeditor");
+    const styleEditor = toolbox.getCurrentPanel();
+    if (!styleEditor) {
+      return;
+    }
+
+    const { url, line, column } = rule.sourceLocation;
+    styleEditor.selectStyleSheet(url, line, column);
+  }
+
   /**
    * Handler for selection events "detached-front" and "new-node-front" and inspector
    * sidbar "select" event. Updates the rules view with the selected node if the panel
@@ -397,6 +429,20 @@ class RulesView {
     }
   }
 
+  /**
+   * Handler for when the toolbox's tools are registered or unregistered.
+   * The source links in the rules view should be enabled only while the
+   * Style Editor is registered because that's where source links point to.
+   */
+  onToolChanged() {
+    const prevIsSourceLinkEnabled = this.store.getState().rules.isSourceLinkEnabled;
+    const isSourceLinkEnabled = this.toolbox.isToolRegistered("styleeditor");
+
+    if (prevIsSourceLinkEnabled !== isSourceLinkEnabled) {
+      this.store.dispatch(updateSourceLinkEnabled(isSourceLinkEnabled));
+    }
+  }
+
   /**
    * Handler for showing the inplace editor when an editable property name is clicked in
    * the rules view.
@@ -550,6 +596,10 @@ class RulesView {
    *         The NodeFront of the current selected element.
    */
   async update(element) {
+    if (this.elementStyle) {
+      this.elementStyle.destroy();
+    }
+
     if (!element) {
       this.store.dispatch(disableAllPseudoClasses());
       this.store.dispatch(updateAddRuleEnabled(false));

devtools/client/inspector/rules/reducers/rules.js

@@ -4,10 +4,14 @@
 
 "use strict";
 
+const Services = require("Services");
+
 const {
   UPDATE_ADD_RULE_ENABLED,
-  UPDATE_RULES,
   UPDATE_HIGHLIGHTED_SELECTOR,
+  UPDATE_RULES,
+  UPDATE_SOURCE_LINK_ENABLED,
+  UPDATE_SOURCE_LINK,
 } = require("../actions/index");
 
 const INITIAL_RULES = {
@@ -15,6 +19,9 @@ const INITIAL_RULES = {
   highlightedSelector: "",
   // Whether or not the add new rule button should be enabled.
   isAddRuleEnabled: false,
+  // Whether or not the source links are enabled. This is determined by
+  // whether or not the style editor is registered.
+  isSourceLinkEnabled: Services.prefs.getBoolPref("devtools.styleeditor.enabled"),
   // Array of CSS rules.
   rules: [],
 };
@@ -113,10 +120,36 @@ const reducers = {
     return {
       highlightedSelector: rules.highlightedSelector,
       isAddRuleEnabled: rules.isAddRuleEnabled,
+      isSourceLinkEnabled: rules.isSourceLinkEnabled,
       rules: newRules.map(rule => getRuleState(rule)),
     };
   },
 
+  [UPDATE_SOURCE_LINK_ENABLED](rules, { enabled }) {
+    return {
+      ...rules,
+      isSourceLinkEnabled: enabled,
+    };
+  },
+
+  [UPDATE_SOURCE_LINK](rules, { ruleId, sourceLink }) {
+    return {
+      highlightedSelector: rules.highlightedSelector,
+      isAddRuleEnabled: rules.isAddRuleEnabled,
+      isSourceLinkEnabled: rules.isSourceLinkEnabled,
+      rules: rules.rules.map(rule => {
+        if (rule.id !== ruleId) {
+          return rule;
+        }
+
+        return {
+          ...rule,
+          sourceLink,
+        };
+      }),
+    };
+  },
+
 };
 
 module.exports = function(rules = INITIAL_RULES, action) {

devtools/client/inspector/rules/types.js

@@ -117,21 +117,6 @@ const selector = exports.selector = {
   selectors: PropTypes.arrayOf(PropTypes.string),
 };
 
-/**
- * A CSS rule's stylesheet source.
- */
-const sourceLink = exports.sourceLink = {
-  // The CSS rule's column number within the stylesheet.
-  column: PropTypes.number,
-  // The CSS rule's line number within the stylesheet.
-  line: PropTypes.number,
-  // The media query text within a @media rule.
-  // Note: Abstract this to support other at-rules in the future.
-  mediaText: PropTypes.string,
-  // The title used for the stylesheet source.
-  title: PropTypes.string,
-};
-
 /**
  * A CSS Rule.
  */
@@ -171,7 +156,12 @@ exports.rule = {
   selector: PropTypes.shape(selector),
 
   // An object containing information about the CSS rule's stylesheet source.
-  sourceLink: PropTypes.shape(sourceLink),
+  sourceLink: PropTypes.shape({
+    // The label used for the stylesheet source
+    label: PropTypes.string,
+    // The title used for the stylesheet source.
+    title: PropTypes.string,
+  }),
 
   // The CSS rule type.
   type: PropTypes.number,

devtools/client/themes/rules.css

@@ -210,11 +210,12 @@
   unicode-bidi: embed
 }
 
-.ruleview-rule-source[unselectable],
+.ruleview-rule-source.disabled > .ruleview-rule-source-label,
 .ruleview-rule-source[unselectable] > .ruleview-rule-source-label {
   cursor: default;
 }
 
+.ruleview-rule-source:not(.unselectable):hover,
 .ruleview-rule-source:not([unselectable]):hover {
   text-decoration: underline;
 }