Bug 1120570 - Fix media query in rule view link text. r=bgrins
authorMichael Ratcliffe <mratcliffe@mozilla.com>
Mon, 23 Mar 2015 17:33:02 +0000
changeset 264254 c8590713b55fb83155faaf88875d4778360f8a28
parent 264253 c37984bc95dbb5d33f763202d39a1f79167ea437
child 264255 d51655b90787a2d37995b53db0e6b8ceb215a03c
push id4718
push userraliiev@mozilla.com
push dateMon, 11 May 2015 18:39:53 +0000
treeherdermozilla-beta@c20c4ef55f08 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbgrins
bugs1120570
milestone39.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 1120570 - Fix media query in rule view link text. r=bgrins
browser/devtools/styleinspector/rule-view.js
browser/devtools/styleinspector/test/browser_ruleview_content_01.js
browser/devtools/styleinspector/test/head.js
toolkit/devtools/server/actors/styles.js
toolkit/devtools/server/actors/stylesheets.js
toolkit/devtools/server/tests/mochitest/test_styles-applied.html
--- a/browser/devtools/styleinspector/rule-view.js
+++ b/browser/devtools/styleinspector/rule-view.js
@@ -430,21 +430,18 @@ function Rule(aElementStyle, aOptions) {
   this.matchedSelectors = aOptions.matchedSelectors || [];
   this.pseudoElement = aOptions.pseudoElement || "";
 
   this.isSystem = aOptions.isSystem;
   this.inherited = aOptions.inherited || null;
   this.keyframes = aOptions.keyframes || null;
   this._modificationDepth = 0;
 
-  if (this.domRule) {
-    let parentRule = this.domRule.parentRule;
-    if (parentRule && parentRule.type == Ci.nsIDOMCSSRule.MEDIA_RULE) {
-      this.mediaText = parentRule.mediaText;
-    }
+  if (this.domRule && this.domRule.mediaText) {
+    this.mediaText = this.domRule.mediaText;
   }
 
   // Populate the text properties with the style's current cssText
   // value, and add in any disabled properties from the store.
   this.textProps = this._getTextProperties();
   this.textProps = this.textProps.concat(this._getDisabledProperties());
 }
 
@@ -502,17 +499,17 @@ Rule.prototype = {
   get sheet() {
     return this.domRule ? this.domRule.parentStyleSheet : null;
   },
 
   /**
    * The rule's line within a stylesheet
    */
   get ruleLine() {
-    return this.domRule ? this.domRule.line : null;
+    return this.domRule ? this.domRule.line : "";
   },
 
   /**
    * The rule's column within a stylesheet
    */
   get ruleColumn() {
     return this.domRule ? this.domRule.column : null;
   },
@@ -524,20 +521,22 @@ Rule.prototype = {
    * @return {Promise}
    *         Promise which resolves with location as an object containing
    *         both the full and short version of the source string.
    */
   getOriginalSourceStrings: function() {
     if (this._originalSourceStrings) {
       return promise.resolve(this._originalSourceStrings);
     }
-    return this.domRule.getOriginalLocation().then(({href, line}) => {
+    return this.domRule.getOriginalLocation().then(({href, line, mediaText}) => {
+      let mediaString = mediaText ? " @" + mediaText : "";
+
       let sourceStrings = {
-        full: href + ":" + line,
-        short: CssLogic.shortSource({href: href}) + ":" + line
+        full: (href || CssLogic.l10n("rule.sourceInline")) + ":" + line + mediaString,
+        short: CssLogic.shortSource({href: href}) + ":" + line + mediaString
       };
 
       this._originalSourceStrings = sourceStrings;
       return sourceStrings;
     }, console.error);
   },
 
   /**
--- a/browser/devtools/styleinspector/test/browser_ruleview_content_01.js
+++ b/browser/devtools/styleinspector/test/browser_ruleview_content_01.js
@@ -7,18 +7,20 @@
 // Test that the rule-view content is correct
 
 add_task(function*() {
   yield addTab("data:text/html;charset=utf-8,browser_ruleview_content.js");
   let {toolbox, inspector, view} = yield openRuleView();
 
   info("Creating the test document");
   let style = "" +
-    "#testid {" +
-    "  background-color: blue;" +
+    "@media screen and (min-width: 10px) {" +
+    "  #testid {" +
+    "    background-color: blue;" +
+    "  }" +
     "}" +
     ".testclass, .unmatched {" +
     "  background-color: green;" +
     "}";
   let styleNode = addStyle(content.document, style);
   content.document.body.innerHTML = "<div id='testid' class='testclass'>Styled Node</div>" +
                                     "<div id='testid2'>Styled Node</div>";
 
@@ -30,14 +32,23 @@ function* testContentAfterNodeSelection(
   is(ruleView.element.querySelectorAll("#noResults").length, 0,
     "After a highlight, no longer has a no-results element.");
 
   yield clearCurrentNodeSelection(inspector)
   is(ruleView.element.querySelectorAll("#noResults").length, 1,
     "After highlighting null, has a no-results element again.");
 
   yield selectNode("#testid", inspector);
+
+  let linkText = getRuleViewLinkTextByIndex(ruleView, 1);
+  is(linkText, "inline:1 @screen and (min-width: 10px)",
+    "link text at index 1 contains media query text.");
+
+  linkText = getRuleViewLinkTextByIndex(ruleView, 2);
+  is(linkText, "inline:1",
+    "link text at index 2 contains no media query text.");
+
   let classEditor = getRuleViewRuleEditor(ruleView, 2);
   is(classEditor.selectorText.querySelector(".ruleview-selector-matched").textContent,
     ".testclass", ".textclass should be matched.");
   is(classEditor.selectorText.querySelector(".ruleview-selector-unmatched").textContent,
     ".unmatched", ".unmatched should not be matched.");
 }
--- a/browser/devtools/styleinspector/test/head.js
+++ b/browser/devtools/styleinspector/test/head.js
@@ -689,16 +689,27 @@ let simulateColorPickerChange = Task.asy
  * @return {DOMNode} The link if any at this index
  */
 function getRuleViewLinkByIndex(view, index) {
   let links = view.doc.querySelectorAll(".ruleview-rule-source");
   return links[index];
 }
 
 /**
+ * Get rule-link text from the rule-view given its index
+ * @param {CssRuleView} view The instance of the rule-view panel
+ * @param {Number} index The index of the link to get
+ * @return {String} The string at this index
+ */
+function getRuleViewLinkTextByIndex(view, index) {
+  let link = getRuleViewLinkByIndex(view, index);
+  return link.querySelector(".source-link-label").value;
+}
+
+/**
  * Get the rule editor from the rule-view given its index
  * @param {CssRuleView} view The instance of the rule-view panel
  * @param {Number} childrenIndex The children index of the element to get
  * @param {Number} nodeIndex The child node index of the element to get
  * @return {DOMNode} The rule editor if any at this index
  */
 function getRuleViewRuleEditor(view, childrenIndex, nodeIndex) {
   return nodeIndex !== undefined ?
--- a/toolkit/devtools/server/actors/styles.js
+++ b/toolkit/devtools/server/actors/styles.js
@@ -986,16 +986,27 @@ var StyleRuleActor = protocol.ActorClass
       actor: this.actorID,
       type: this.type,
       line: this.line || undefined,
       column: this.column
     };
 
     if (this.rawRule.parentRule) {
       form.parentRule = this.pageStyle._styleRef(this.rawRule.parentRule).actorID;
+
+      // CSS rules that we call media rules are STYLE_RULES that are children
+      // of MEDIA_RULEs. We need to check the parentRule to check if a rule is
+      // a media rule so we do this here instead of in the switch statement
+      // below.
+      if (this.rawRule.parentRule.type === Ci.nsIDOMCSSRule.MEDIA_RULE) {
+        form.media = [];
+        for (let i = 0, n = this.rawRule.parentRule.media.length; i < n; i++) {
+          form.media.push(this.rawRule.parentRule.media.item(i));
+        }
+      }
     }
     if (this.rawRule.parentStyleSheet) {
       form.parentStyleSheet = this.pageStyle._sheetRef(this.rawRule.parentStyleSheet).actorID;
     }
 
     switch (this.type) {
       case Ci.nsIDOMCSSRule.STYLE_RULE:
         form.selectors = CssLogic.getSelectors(this.rawRule);
@@ -1009,22 +1020,16 @@ var StyleRuleActor = protocol.ActorClass
         form.cssText = this.rawStyle.cssText || "";
         break;
       case Ci.nsIDOMCSSRule.CHARSET_RULE:
         form.encoding = this.rawRule.encoding;
         break;
       case Ci.nsIDOMCSSRule.IMPORT_RULE:
         form.href = this.rawRule.href;
         break;
-      case Ci.nsIDOMCSSRule.MEDIA_RULE:
-        form.media = [];
-        for (let i = 0, n = this.rawRule.media.length; i < n; i++) {
-          form.media.push(this.rawRule.media.item(i));
-        }
-        break;
       case Ci.nsIDOMCSSRule.KEYFRAMES_RULE:
         form.cssText = this.rawRule.cssText;
         form.name = this.rawRule.name;
         break;
       case Ci.nsIDOMCSSRule.KEYFRAME_RULE:
         form.cssText = this.rawStyle.cssText || "";
         form.keyText = this.rawRule.keyText || "";
         break;
@@ -1240,37 +1245,39 @@ var StyleRuleFront = protocol.FrontClass
     };
   },
 
   getOriginalLocation: function()
   {
     if (this._originalLocation) {
       return promise.resolve(this._originalLocation);
     }
-
     let parentSheet = this.parentStyleSheet;
     if (!parentSheet) {
+      // This rule doesn't belong to a stylesheet so it is an inline style.
+      // Inline styles do not have any mediaText so we can return early.
       return promise.resolve(this.location);
     }
     return parentSheet.getOriginalLocation(this.line, this.column)
       .then(({ fromSourceMap, source, line, column }) => {
         let location = {
           href: source,
           line: line,
-          column: column
-        }
+          column: column,
+          mediaText: this.mediaText
+        };
         if (fromSourceMap === false) {
           location.source = this.parentStyleSheet;
         }
         if (!source) {
           location.href = this.href;
         }
         this._originalLocation = location;
         return location;
-      })
+      });
   }
 });
 
 /**
  * Convenience API for building a list of attribute modifications
  * for the `modifyAttributes` request.
  */
 var RuleModificationList = Class({
--- a/toolkit/devtools/server/actors/stylesheets.js
+++ b/toolkit/devtools/server/actors/stylesheets.js
@@ -739,17 +739,17 @@ let StyleSheetActor = protocol.ActorClas
       if (sourceMap) {
         return sourceMap.originalPositionFor({ line: line, column: column });
       }
       return {
         fromSourceMap: false,
         source: this.href,
         line: line,
         column: column
-      }
+      };
     });
   }, {
     request: {
       line: Arg(0, "number"),
       column: Arg(1, "number")
     },
     response: RetVal(types.addDictType("originallocationresponse", {
       source: "string",
--- a/toolkit/devtools/server/tests/mochitest/test_styles-applied.html
+++ b/toolkit/devtools/server/tests/mochitest/test_styles-applied.html
@@ -117,17 +117,17 @@ addTest(function matchedSelectors() {
 addTest(function testMediaQuery() {
   promiseDone(gWalker.querySelector(gWalker.rootNode, "#mediaqueried").then(node => {
     return gStyles.getApplied(node, {
       inherited: false, filter: "user", matchedSelectors: true
     });
   }).then(applied => {
     is(applied[1].rule.type, 1, "Entry 1 is a rule style");
     is(applied[1].rule.parentRule.type, 4, "Entry 1's parent rule is a media rule");
-    is(applied[1].rule.parentRule.media[0], "screen", "Entry 1's parent rule has the expected medium");
+    is(applied[1].rule.media[0], "screen", "Entry 1's rule has the expected medium");
   }).then(runNextTest));
 });
 
 addTest(function cleanup() {
   delete gStyles;
   delete gWalker;
   delete gClient;
   runNextTest();