Bug 1503924 - Mark style sources from iframes in the Changes panel. r=gl
authorRazvan Caliman <rcaliman@mozilla.com>
Mon, 26 Nov 2018 05:02:06 +0000
changeset 504766 677d32e4a6afc448e3d6c8f359cb1161c906181e
parent 504765 999546476a96355dd32bc1e3cdd230b8f58521c4
child 504767 a0b8137e55edab833c4d0c472cb1a7f8f13f7840
push id10290
push userffxbld-merge
push dateMon, 03 Dec 2018 16:23:23 +0000
treeherdermozilla-beta@700bed2445e6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgl
bugs1503924
milestone65.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1503924 - Mark style sources from iframes in the Changes panel. r=gl Depends on D12590 Differential Revision: https://phabricator.services.mozilla.com/D12697
devtools/client/inspector/changes/components/ChangesApp.js
devtools/client/inspector/changes/reducers/changes.js
devtools/client/inspector/changes/utils/changes-utils.js
devtools/client/locales/en-US/changes.properties
devtools/client/themes/changes.css
devtools/server/actors/styles.js
--- a/devtools/client/inspector/changes/components/ChangesApp.js
+++ b/devtools/client/inspector/changes/components/ChangesApp.js
@@ -105,38 +105,49 @@ class ChangesApp extends PureComponent {
       dom.div({ className: `level bracket-close ${diffClass}` }, "}")
     );
   }
 
   renderDiff(changes = {}) {
     // Render groups of style sources: stylesheets and element style attributes.
     return Object.entries(changes).map(([sourceId, source]) => {
       const path = getSourceForDisplay(source);
-      const { href, rules } = source;
+      const { href, rules, isFramed } = source;
 
       return dom.div(
         {
           key: sourceId,
           className: "source",
         },
         dom.div(
           {
             className: "href",
             title: href,
           },
-          path
+          dom.span({}, path),
+          isFramed && this.renderFrameBadge(href)
         ),
         // Render changed rules within this source.
         Object.entries(rules).map(([ruleId, rule]) => {
           return this.renderRule(ruleId, rule, rules);
         })
       );
     });
   }
 
+  renderFrameBadge(href = "") {
+    return dom.span(
+      {
+        className: "inspector-badge",
+        title: href,
+      },
+      getStr("changes.iframeLabel")
+    );
+  }
+
   renderEmptyState() {
     return dom.div({ className: "devtools-sidepanel-no-result" },
       dom.p({}, getStr("changes.noChanges")),
       dom.p({}, getStr("changes.noChangesDescription"))
     );
   }
 
   render() {
--- a/devtools/client/inspector/changes/reducers/changes.js
+++ b/devtools/client/inspector/changes/reducers/changes.js
@@ -187,23 +187,23 @@ const reducers = {
       ancestors: [],
       add: [],
       remove: [],
     };
 
     change = { ...defaults, ...change };
     state = cloneState(state);
 
-    const { type, href, index } = change.source;
+    const { type, href, index, isFramed } = change.source;
     const { selector, ancestors, ruleIndex, type: changeType } = change;
     const sourceId = getSourceHash(change.source);
     const ruleId = getRuleHash({ selector, ancestors, ruleIndex });
 
     // Copy or create object identifying the source (styelsheet/element) for this change.
-    const source = Object.assign({}, state[sourceId], { type, href, index });
+    const source = Object.assign({}, state[sourceId], { type, href, index, isFramed });
     // Copy or create collection of all rules ever changed in this source.
     const rules = Object.assign({}, source.rules);
     // Refrence or create object identifying the rule for this change.
     let rule = rules[ruleId];
     if (!rule) {
       rule = createRule({ selector, ancestors, ruleIndex }, rules);
       if (changeType.startsWith("rule-")) {
         rule.changeType = changeType;
--- a/devtools/client/inspector/changes/utils/changes-utils.js
+++ b/devtools/client/inspector/changes/utils/changes-utils.js
@@ -8,29 +8,29 @@ const { getFormatStr, getStr } = require
 
 /**
 * Generate a hash that uniquely identifies a stylesheet or element style attribute.
 *
 * @param {Object} source
 *        Information about a stylesheet or element style attribute:
 *        {
 *          type:  {String}
-*                 One of "stylesheet" or "element".
+*                 One of "stylesheet", "inline" or "element".
 *          index: {Number|String}
 *                 Position of the styleshet in the list of stylesheets in the document.
 *                 If `type` is "element", `index` is the generated selector which
 *                 uniquely identifies the element in the document.
-*          href:  {String|null}
-*                 URL of the stylesheet or of the document when `type` is "element".
-*                 If the stylesheet is inline, `href` is null.
+*          href:  {String}
+*                 URL of the stylesheet or of the document when `type` is "element" or
+*                 "inline".
 *        }
 * @return {String}
 */
 function getSourceHash(source) {
-  const { type, index, href = "inline" } = source;
+  const { type, index, href } = source;
 
   return `${type}${index}${href}`;
 }
 
 /**
 * Generate a hash that uniquely identifies a CSS rule.
 *
 * @param {Object} ruleData
@@ -71,23 +71,22 @@ function getRuleHash(ruleData) {
  */
 function getSourceForDisplay(source) {
   let href;
 
   switch (source.type) {
     case "element":
       href = getStr("changes.elementStyleLabel");
       break;
+    case "inline":
+      href = getFormatStr("changes.inlineStyleSheetLabel", `#${source.index}`);
+      break;
     case "stylesheet":
-      if (source.href) {
-        const url = new URL(source.href);
-        href = url.pathname.substring(url.pathname.lastIndexOf("/") + 1);
-      } else {
-        href = getFormatStr("changes.inlineStyleSheetLabel", `#${source.index}`);
-      }
+      const url = new URL(source.href);
+      href = url.pathname.substring(url.pathname.lastIndexOf("/") + 1);
       break;
   }
 
   return href;
 }
 
 module.exports.getSourceForDisplay = getSourceForDisplay;
 module.exports.getSourceHash = getSourceHash;
--- a/devtools/client/locales/en-US/changes.properties
+++ b/devtools/client/locales/en-US/changes.properties
@@ -15,8 +15,12 @@ changes.noChangesDescription=Changes to 
 # LOCALIZATION NOTE (changes.inlineStyleSheetLabel): This label appears in the Changes
 # panel above changes done to inline stylesheets. The variable will be replaced with the
 # index of the stylesheet within its document like so: Inline #1
 changes.inlineStyleSheetLabel=Inline %S
 
 # LOCALIZATION NOTE (changes.elementStyleLabel): This label appears in the Changes
 # panel above changes done to element styles.
 changes.elementStyleLabel=Element
+
+# LOCALIZATION NOTE (changes.iframeLabel): This label appears next to URLs of stylesheets
+# and element inline styles hosted by iframes. Lowercase intentional.
+changes.iframeLabel=iframe
--- a/devtools/client/themes/changes.css
+++ b/devtools/client/themes/changes.css
@@ -21,22 +21,29 @@
   margin: 0;
   padding: 0;
   width: 100%;
   height: 100%;
   overflow: auto;
 }
 
 #sidebar-panel-changes .href {
+  display: flex;
+  align-items: center;
   color: var(--theme-toolbar-color);
   background: var(--theme-toolbar-background);
   border-top: 1px solid var(--theme-splitter-color);
   border-bottom: 1px solid var(--theme-splitter-color);
   padding: 4px;
   font-size: 12px;
+}
+
+#sidebar-panel-changes .href span {
+  /* Allows trimming of flex item with overflow ellipsis within the flex container */
+  min-width: 0;
   white-space: nowrap;
   text-overflow: ellipsis;
   overflow: hidden;
 }
 
 #sidebar-panel-changes .level {
   padding-top: 3px;
   padding-right: 5px;
--- a/devtools/server/actors/styles.js
+++ b/devtools/server/actors/styles.js
@@ -105,16 +105,20 @@ var PageStyleActor = protocol.ActorClass
     }
     this._watchedSheets.clear();
   },
 
   get conn() {
     return this.inspector.conn;
   },
 
+  get ownerWindow() {
+    return this.inspector.targetActor.window;
+  },
+
   form: function(detail) {
     if (detail === "actorid") {
       return this.actorID;
     }
 
     // We need to use CSS from the inspected window in order to use CSS.supports() and
     // detect the right platform features from there.
     const CSS = this.inspector.targetActor.window.CSS;
@@ -1122,26 +1126,31 @@ var StyleRuleActor = protocol.ActorClass
 
       data.source = {
         type: "element",
         // Used to differentiate between elements which match the same generated selector
         // but live in different documents (ex: host document and iframe).
         href: this.rawNode.baseURI,
         // Element style attributes don't have a rule index; use the generated selector.
         index: data.selector,
+        // Whether the element lives in a different frame than the host document.
+        isFramed: this.rawNode.ownerGlobal !== this.pageStyle.ownerWindow,
       };
       data.ruleIndex = 0;
     } else {
       data.selector = (this.type === CSSRule.KEYFRAME_RULE)
         ? this.rawRule.keyText
         : this.rawRule.selectorText;
       data.source = {
-        type: "stylesheet",
-        href: this.sheetActor.href,
+        // Inline stylesheets have a null href; Use window URL instead.
+        type: this.sheetActor.href ? "stylesheet" : "inline",
+        href: this.sheetActor.href || this.sheetActor.window.location.toString(),
         index: this.sheetActor.styleSheetIndex,
+        // Whether the stylesheet lives in a different frame than the host document.
+        isFramed: this.sheetActor.ownerWindow !== this.sheetActor.window,
       };
       // Used to differentiate between changes to rules with identical selectors.
       data.ruleIndex = this._ruleIndex;
     }
 
     return data;
   },