From 0f39585f1706f2f9c50d5f7152dd39df017a5144 Mon Sep 17 00:00:00 2001 From: Michael Ratcliffe Date: Thu, 1 Mar 2012 19:13:12 +0000 Subject: [PATCH] Bug 722196 - Rule view showing rules in the wrong order; r=jwalker --- browser/devtools/styleinspector/CssLogic.jsm | 15 ++++ .../devtools/styleinspector/CssRuleView.jsm | 12 +++- .../devtools/styleinspector/test/Makefile.in | 3 + ...wser_bug722196_identify_media_queries.html | 24 +++++++ ...r_bug722196_property_view_media_queries.js | 68 +++++++++++++++++++ ...owser_bug722196_rule_view_media_queries.js | 55 +++++++++++++++ .../gnomestripe/devtools/csshtmltree.css | 1 + .../themes/pinstripe/devtools/csshtmltree.css | 1 + .../themes/winstripe/devtools/csshtmltree.css | 1 + 9 files changed, 179 insertions(+), 1 deletion(-) create mode 100644 browser/devtools/styleinspector/test/browser_bug722196_identify_media_queries.html create mode 100644 browser/devtools/styleinspector/test/browser_bug722196_property_view_media_queries.js create mode 100644 browser/devtools/styleinspector/test/browser_bug722196_rule_view_media_queries.js diff --git a/browser/devtools/styleinspector/CssLogic.jsm b/browser/devtools/styleinspector/CssLogic.jsm index d450536fc276..0a431db857d4 100644 --- a/browser/devtools/styleinspector/CssLogic.jsm +++ b/browser/devtools/styleinspector/CssLogic.jsm @@ -1198,11 +1198,19 @@ function CssRule(aCssSheet, aDomRule, aElement) this._cssSheet = aCssSheet; this._domRule = aDomRule; + let parentRule = aDomRule.parentRule; + if (parentRule && parentRule.type == Ci.nsIDOMCSSRule.MEDIA_RULE) { + this.mediaText = parentRule.media.mediaText; + } + if (this._cssSheet) { // parse _domRule.selectorText on call to this.selectors this._selectors = null; this.line = this._cssSheet._cssLogic.domUtils.getRuleLine(this._domRule); this.source = this._cssSheet.shortSource + ":" + this.line; + if (this.mediaText) { + this.source += " @media " + this.mediaText; + } this.href = this._cssSheet.href; this.contentRule = this._cssSheet.contentSheet; } else if (aElement) { @@ -1218,6 +1226,13 @@ function CssRule(aCssSheet, aDomRule, aElement) CssRule.prototype = { _passId: null, + mediaText: "", + + get isMediaRule() + { + return !!this.mediaText; + }, + /** * Check if the parent stylesheet is allowed by the CssLogic.sourceFilter. * diff --git a/browser/devtools/styleinspector/CssRuleView.jsm b/browser/devtools/styleinspector/CssRuleView.jsm index 9d179a30dbea..64d92e703ddc 100644 --- a/browser/devtools/styleinspector/CssRuleView.jsm +++ b/browser/devtools/styleinspector/CssRuleView.jsm @@ -356,10 +356,20 @@ function Rule(aElementStyle, aOptions) this.style = aOptions.style || this.domRule.style; this.selectorText = aOptions.selectorText || this.domRule.selectorText; this.inherited = aOptions.inherited || null; + + if (this.domRule) { + let parentRule = this.domRule.parentRule; + if (parentRule && parentRule.type == Ci.nsIDOMCSSRule.MEDIA_RULE) { + this.mediaText = parentRule.media.mediaText; + } + } + this._getTextProperties(); } Rule.prototype = { + mediaText: "", + get title() { if (this._title) { @@ -380,7 +390,7 @@ Rule.prototype = { args, args.length); } - return this._title; + return this._title + (this.mediaText ? " @media " + this.mediaText : ""); }, /** diff --git a/browser/devtools/styleinspector/test/Makefile.in b/browser/devtools/styleinspector/test/Makefile.in index a313cb6b630d..031a9e83eee2 100644 --- a/browser/devtools/styleinspector/test/Makefile.in +++ b/browser/devtools/styleinspector/test/Makefile.in @@ -61,6 +61,8 @@ _BROWSER_TEST_FILES = \ browser_ruleview_override.js \ browser_ruleview_ui.js \ browser_bug705707_is_content_stylesheet.js \ + browser_bug722196_property_view_media_queries.js \ + browser_bug722196_rule_view_media_queries.js \ browser_bug_592743_specificity.js \ head.js \ $(NULL) @@ -74,6 +76,7 @@ _BROWSER_TEST_PAGES = \ browser_bug705707_is_content_stylesheet_script.css \ browser_bug705707_is_content_stylesheet.xul \ browser_bug705707_is_content_stylesheet_xul.css \ + browser_bug722196_identify_media_queries.html \ $(NULL) libs:: $(_BROWSER_TEST_FILES) diff --git a/browser/devtools/styleinspector/test/browser_bug722196_identify_media_queries.html b/browser/devtools/styleinspector/test/browser_bug722196_identify_media_queries.html new file mode 100644 index 000000000000..1adb8bc7a731 --- /dev/null +++ b/browser/devtools/styleinspector/test/browser_bug722196_identify_media_queries.html @@ -0,0 +1,24 @@ + + + test + + + + +
+ + diff --git a/browser/devtools/styleinspector/test/browser_bug722196_property_view_media_queries.js b/browser/devtools/styleinspector/test/browser_bug722196_property_view_media_queries.js new file mode 100644 index 000000000000..fa1602b23eca --- /dev/null +++ b/browser/devtools/styleinspector/test/browser_bug722196_property_view_media_queries.js @@ -0,0 +1,68 @@ +/* vim: set ts=2 et sw=2 tw=80: */ +/* Any copyright is dedicated to the Public Domain. + http://creativecommons.org/publicdomain/zero/1.0/ */ + +// Tests that we correctly display appropriate media query titles in the +// property view. + +let doc; +let stylePanel; + +const TEST_URI = "http://example.com/browser/browser/devtools/styleinspector/" + + "test/browser_bug722196_identify_media_queries.html"; + +function test() +{ + waitForExplicitFinish(); + addTab(TEST_URI); + browser.addEventListener("load", docLoaded, true); +} + +function docLoaded() +{ + browser.removeEventListener("load", docLoaded, true); + doc = content.document; + stylePanel = new StyleInspector(window); + Services.obs.addObserver(checkSheets, "StyleInspector-opened", false); + stylePanel.createPanel(false, function() { + stylePanel.open(doc.body); + }); +} + +function checkSheets() +{ + Services.obs.removeObserver(checkSheets, "StyleInspector-opened", false); + + ok(stylePanel.isOpen(), "style inspector is open"); + + var div = doc.querySelector("div"); + ok(div, "captain, we have the div"); + + stylePanel.selectNode(div); + + let cssLogic = stylePanel.cssLogic; + cssLogic.processMatchedSelectors(); + + let _strings = Services.strings + .createBundle("chrome://browser/locale/devtools/styleinspector.properties"); + + let inline = _strings.GetStringFromName("rule.sourceInline"); + + let source1 = inline + ":8"; + let source2 = inline + ":15 @media screen and (min-width: 1px)"; + is(cssLogic._matchedRules[0][0].source, source1, + "rule.source gives correct output for rule 1"); + is(cssLogic._matchedRules[1][0].source, source2, + "rule.source gives correct output for rule 2"); + + Services.obs.addObserver(finishUp, "StyleInspector-closed", false); + stylePanel.close(); +} + +function finishUp() +{ + Services.obs.removeObserver(finishUp, "StyleInspector-closed", false); + doc = null; + gBrowser.removeCurrentTab(); + finish(); +} diff --git a/browser/devtools/styleinspector/test/browser_bug722196_rule_view_media_queries.js b/browser/devtools/styleinspector/test/browser_bug722196_rule_view_media_queries.js new file mode 100644 index 000000000000..9ba647ce9598 --- /dev/null +++ b/browser/devtools/styleinspector/test/browser_bug722196_rule_view_media_queries.js @@ -0,0 +1,55 @@ +/* vim: set ts=2 et sw=2 tw=80: */ +/* Any copyright is dedicated to the Public Domain. + http://creativecommons.org/publicdomain/zero/1.0/ */ + +// Tests that we correctly display appropriate media query titles in the +// rule view. + +let tempScope = {}; +Cu.import("resource:///modules/devtools/CssRuleView.jsm", tempScope); +let _ElementStyle = tempScope._ElementStyle; +let doc; + +const TEST_URI = "http://example.com/browser/browser/devtools/styleinspector/" + + "test/browser_bug722196_identify_media_queries.html"; + +function test() +{ + waitForExplicitFinish(); + addTab(TEST_URI); + browser.addEventListener("load", docLoaded, true); +} + +function docLoaded() +{ + browser.removeEventListener("load", docLoaded, true); + doc = content.document; + checkSheets(); +} + +function checkSheets() +{ + var div = doc.querySelector("div"); + ok(div, "captain, we have the div"); + + let elementStyle = new _ElementStyle(div); + is(elementStyle.rules.length, 3, "Should have 3 rules."); + + let _strings = Services.strings + .createBundle("chrome://browser/locale/devtools/styleinspector.properties"); + + let inline = _strings.GetStringFromName("rule.sourceInline"); + + is(elementStyle.rules[0].title, inline, "check rule 0 title"); + is(elementStyle.rules[1].title, inline + + ":15 @media screen and (min-width: 1px)", "check rule 1 title"); + is(elementStyle.rules[2].title, inline + ":8", "check rule 2 title"); + finishUp(); +} + +function finishUp() +{ + doc = null; + gBrowser.removeCurrentTab(); + finish(); +} diff --git a/browser/themes/gnomestripe/devtools/csshtmltree.css b/browser/themes/gnomestripe/devtools/csshtmltree.css index 738130a17f56..0ee9c461260e 100644 --- a/browser/themes/gnomestripe/devtools/csshtmltree.css +++ b/browser/themes/gnomestripe/devtools/csshtmltree.css @@ -146,6 +146,7 @@ direction: ltr; padding: 0; -moz-padding-start: 20px; + vertical-align: text-bottom; } .bestmatch { diff --git a/browser/themes/pinstripe/devtools/csshtmltree.css b/browser/themes/pinstripe/devtools/csshtmltree.css index 8593a5858450..e40f46af7247 100644 --- a/browser/themes/pinstripe/devtools/csshtmltree.css +++ b/browser/themes/pinstripe/devtools/csshtmltree.css @@ -148,6 +148,7 @@ direction: ltr; padding: 0; -moz-padding-start: 20px; + vertical-align: text-bottom; } .bestmatch { diff --git a/browser/themes/winstripe/devtools/csshtmltree.css b/browser/themes/winstripe/devtools/csshtmltree.css index 3448e8f6076b..42dce67568cc 100644 --- a/browser/themes/winstripe/devtools/csshtmltree.css +++ b/browser/themes/winstripe/devtools/csshtmltree.css @@ -146,6 +146,7 @@ direction: ltr; padding: 0; -moz-padding-start: 20px; + vertical-align: text-bottom; } .bestmatch {