Bug 1174407 - Collapse pseudo-elements section in rule-view by default; r=miker
authorPatrick Brosset <pbrosset@mozilla.com>
Thu, 18 Jun 2015 10:10:53 +0200
changeset 280315 2d6105c727c87317803321a8544e39817ba02d92
parent 280314 dbbcc1bbe42dada0fb2cddd0b16f53b44c94318a
child 280316 b6340f3f7bf91972c05e3a021355729af16e2dd8
push id4932
push userjlund@mozilla.com
push dateMon, 10 Aug 2015 18:23:06 +0000
treeherdermozilla-beta@6dd5a4f5f745 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmiker
bugs1174407
milestone41.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 1174407 - Collapse pseudo-elements section in rule-view by default; r=miker This changes the default value for the devtools.inspector.show_pseudo_elements pref to false so that the pseudo-elements section in the rule-view is collapsed by default. This section may now contain a lot more rules, especially when showing browser styles and so it's better to have it collapsed by default. This required to fix 2 tests that assumed the section was expanded by default. browser_ruleview_pseudo-element_01.js was also cleaned up a bit to be more compliant with our eslint rules.
browser/app/profile/firefox.js
browser/devtools/styleinspector/test/browser_ruleview_edit-selector_02.js
browser/devtools/styleinspector/test/browser_ruleview_pseudo-element_01.js
--- a/browser/app/profile/firefox.js
+++ b/browser/app/profile/firefox.js
@@ -1404,18 +1404,18 @@ pref("devtools.command-button-rulers.ena
 // Inspector preferences
 // Enable the Inspector
 pref("devtools.inspector.enabled", true);
 // What was the last active sidebar in the inspector
 pref("devtools.inspector.activeSidebar", "ruleview");
 // Enable the markup preview
 pref("devtools.inspector.markupPreview", false);
 pref("devtools.inspector.remote", false);
-// Expand pseudo-elements by default in the rule-view
-pref("devtools.inspector.show_pseudo_elements", true);
+// Collapse pseudo-elements by default in the rule-view
+pref("devtools.inspector.show_pseudo_elements", false);
 // The default size for image preview tooltips in the rule-view/computed-view/markup-view
 pref("devtools.inspector.imagePreviewTooltipSize", 300);
 // Enable user agent style inspection in rule-view
 pref("devtools.inspector.showUserAgentStyles", false);
 // Show all native anonymous content (like controls in <video> tags)
 pref("devtools.inspector.showAllAnonymousContent", false);
 // Enable the MDN docs tooltip
 pref("devtools.inspector.mdnDocsTooltip.enabled", true);
--- a/browser/devtools/styleinspector/test/browser_ruleview_edit-selector_02.js
+++ b/browser/devtools/styleinspector/test/browser_ruleview_edit-selector_02.js
@@ -17,17 +17,22 @@ let PAGE_CONTENT = [
   '  }',
   '</style>',
   '<div id="testid">Styled Node</div>',
   '<span class="testclass">This is a span</span>',
   '<div class="testclass2">A</div>',
   '<div id="testid3">B</div>'
 ].join("\n");
 
+const PSEUDO_PREF = "devtools.inspector.show_pseudo_elements";
+
 add_task(function*() {
+  // Expand the pseudo-elements section by default.
+  Services.prefs.setBoolPref(PSEUDO_PREF, true);
+
   yield addTab("data:text/html;charset=utf-8," + encodeURIComponent(PAGE_CONTENT));
   let {inspector, view} = yield openRuleView();
 
   info("Selecting the test element");
   yield selectNode(".testclass", inspector);
   yield testEditSelector(view, "div:nth-child(1)");
 
   info("Selecting the modified element");
@@ -36,16 +41,19 @@ add_task(function*() {
 
   info("Selecting the test element");
   yield selectNode("#testid3", inspector);
   yield testEditSelector(view, ".testclass2::first-letter");
 
   info("Selecting the modified element");
   yield selectNode(".testclass2", inspector);
   yield checkModifiedElement(view, ".testclass2::first-letter");
+
+  // Reset the pseudo-elements section pref to its default value.
+  Services.prefs.clearUserPref(PSEUDO_PREF);
 });
 
 function* testEditSelector(view, name) {
   info("Test editing existing selector fields");
 
   let idRuleEditor = getRuleViewRuleEditor(view, 1) ||
     getRuleViewRuleEditor(view, 1, 0);
 
--- a/browser/devtools/styleinspector/test/browser_ruleview_pseudo-element_01.js
+++ b/browser/devtools/styleinspector/test/browser_ruleview_pseudo-element_01.js
@@ -2,134 +2,139 @@
 /* Any copyright is dedicated to the Public Domain.
  http://creativecommons.org/publicdomain/zero/1.0/ */
 
 "use strict";
 
 // Test that pseudoelements are displayed correctly in the rule view
 
 const TEST_URI = TEST_URL_ROOT + "doc_pseudoelement.html";
+const PSEUDO_PREF = "devtools.inspector.show_pseudo_elements";
 
 add_task(function*() {
+  Services.prefs.setBoolPref(PSEUDO_PREF, true);
+
   yield addTab(TEST_URI);
-  let {toolbox, inspector, view} = yield openRuleView();
+  let {inspector, view} = yield openRuleView();
 
   yield testTopLeft(inspector, view);
   yield testTopRight(inspector, view);
   yield testBottomRight(inspector, view);
   yield testBottomLeft(inspector, view);
   yield testParagraph(inspector, view);
   yield testBody(inspector, view);
+
+  Services.prefs.clearUserPref(PSEUDO_PREF);
 });
 
 function* testTopLeft(inspector, view) {
   let selector = "#topleft";
-  let {
-    rules,
-    element,
-    elementStyle
-  } = yield assertPseudoElementRulesNumbers(selector, inspector, view, {
+  let {rules} = yield assertPseudoElementRulesNumbers(selector, inspector, view, {
     elementRulesNb: 4,
     firstLineRulesNb: 2,
     firstLetterRulesNb: 1,
     selectionRulesNb: 0
   });
 
   let gutters = assertGutters(view);
 
-  // Make sure that clicking on the twisty hides pseudo elements
+  info("Make sure that clicking on the twisty hides pseudo elements");
   let expander = gutters[0].querySelector(".ruleview-expander");
-  ok (view.element.firstChild.classList.contains("show-expandable-container"), "Pseudo Elements are expanded");
+  ok(view.element.firstChild.classList.contains("show-expandable-container"),
+     "Pseudo Elements are expanded");
+
   expander.click();
-  ok (!view.element.firstChild.classList.contains("show-expandable-container"), "Pseudo Elements are collapsed by twisty");
+  ok(!view.element.firstChild.classList.contains("show-expandable-container"),
+     "Pseudo Elements are collapsed by twisty");
+
   expander.click();
-  ok (view.element.firstChild.classList.contains("show-expandable-container"), "Pseudo Elements are expanded again");
+  ok(view.element.firstChild.classList.contains("show-expandable-container"),
+     "Pseudo Elements are expanded again");
 
-  // Make sure that dblclicking on the header container also toggles the pseudo elements
-  EventUtils.synthesizeMouseAtCenter(gutters[0], {clickCount: 2}, inspector.sidebar.getWindowForTab("ruleview"));
-  ok (!view.element.firstChild.classList.contains("show-expandable-container"), "Pseudo Elements are collapsed by dblclicking");
-
-  let defaultView = element.ownerDocument.defaultView;
+  info("Make sure that dblclicking on the header container also toggles " +
+       "the pseudo elements");
+  EventUtils.synthesizeMouseAtCenter(gutters[0], {clickCount: 2},
+                                     view.doc.defaultView);
+  ok(!view.element.firstChild.classList.contains("show-expandable-container"),
+     "Pseudo Elements are collapsed by dblclicking");
 
   let elementRule = rules.elementRules[0];
   let elementRuleView = getRuleViewRuleEditor(view, 3);
 
   let elementFirstLineRule = rules.firstLineRules[0];
-  let elementFirstLineRuleView = [].filter.call(view.element.children[1].children, (e) => {
+  let elementFirstLineRuleView = [...view.element.children[1].children].filter(e => {
     return e._ruleEditor && e._ruleEditor.rule === elementFirstLineRule;
   })[0]._ruleEditor;
 
-  is
-  (
-    convertTextPropsToString(elementFirstLineRule.textProps),
-    "color: orange",
-    "TopLeft firstLine properties are correct"
-  );
+  is(convertTextPropsToString(elementFirstLineRule.textProps),
+     "color: orange",
+     "TopLeft firstLine properties are correct");
 
   let firstProp = elementFirstLineRuleView.addProperty("background-color", "rgb(0, 255, 0)", "");
   let secondProp = elementFirstLineRuleView.addProperty("font-style", "italic", "");
 
-  is (firstProp, elementFirstLineRule.textProps[elementFirstLineRule.textProps.length - 2],
-      "First added property is on back of array");
-  is (secondProp, elementFirstLineRule.textProps[elementFirstLineRule.textProps.length - 1],
-      "Second added property is on back of array");
+  is(firstProp,
+     elementFirstLineRule.textProps[elementFirstLineRule.textProps.length - 2],
+     "First added property is on back of array");
+  is(secondProp,
+     elementFirstLineRule.textProps[elementFirstLineRule.textProps.length - 1],
+     "Second added property is on back of array");
 
   yield elementFirstLineRule._applyingModifications;
 
   is((yield getComputedStyleProperty(selector, ":first-line", "background-color")),
-    "rgb(0, 255, 0)", "Added property should have been used.");
+     "rgb(0, 255, 0)", "Added property should have been used.");
   is((yield getComputedStyleProperty(selector, ":first-line", "font-style")),
-    "italic", "Added property should have been used.");
+     "italic", "Added property should have been used.");
   is((yield getComputedStyleProperty(selector, null, "text-decoration")),
-    "none", "Added property should not apply to element");
+     "none", "Added property should not apply to element");
 
   firstProp.setEnabled(false);
   yield elementFirstLineRule._applyingModifications;
 
   is((yield getComputedStyleProperty(selector, ":first-line", "background-color")),
-    "rgb(255, 0, 0)", "Disabled property should now have been used.");
+     "rgb(255, 0, 0)", "Disabled property should now have been used.");
   is((yield getComputedStyleProperty(selector, null, "background-color")),
-    "rgb(221, 221, 221)", "Added property should not apply to element");
+     "rgb(221, 221, 221)", "Added property should not apply to element");
 
   firstProp.setEnabled(true);
   yield elementFirstLineRule._applyingModifications;
 
   is((yield getComputedStyleProperty(selector, ":first-line", "background-color")),
-    "rgb(0, 255, 0)", "Added property should have been used.");
+     "rgb(0, 255, 0)", "Added property should have been used.");
   is((yield getComputedStyleProperty(selector, null, "text-decoration")),
-    "none", "Added property should not apply to element");
+     "none", "Added property should not apply to element");
 
   firstProp = elementRuleView.addProperty("background-color", "rgb(0, 0, 255)", "");
   yield elementRule._applyingModifications;
 
   is((yield getComputedStyleProperty(selector, null, "background-color")),
-    "rgb(0, 0, 255)", "Added property should have been used.");
+     "rgb(0, 0, 255)", "Added property should have been used.");
   is((yield getComputedStyleProperty(selector, ":first-line", "background-color")),
-    "rgb(0, 255, 0)", "Added prop does not apply to pseudo");
+     "rgb(0, 255, 0)", "Added prop does not apply to pseudo");
 }
 
 function* testTopRight(inspector, view) {
-  let {
-    rules,
-    element,
-    elementStyle
-  } = yield assertPseudoElementRulesNumbers("#topright", inspector, view, {
+  yield assertPseudoElementRulesNumbers("#topright", inspector, view, {
     elementRulesNb: 4,
     firstLineRulesNb: 1,
     firstLetterRulesNb: 1,
     selectionRulesNb: 0
   });
 
   let gutters = assertGutters(view);
 
   let expander = gutters[0].querySelector(".ruleview-expander");
-  ok (!view.element.firstChild.classList.contains("show-expandable-container"), "Pseudo Elements remain collapsed after switching element");
+  ok(!view.element.firstChild.classList.contains("show-expandable-container"),
+     "Pseudo Elements remain collapsed after switching element");
+
   expander.scrollIntoView();
   expander.click();
-  ok (view.element.firstChild.classList.contains("show-expandable-container"), "Pseudo Elements are shown again after clicking twisty");
+  ok(view.element.firstChild.classList.contains("show-expandable-container"),
+     "Pseudo Elements are shown again after clicking twisty");
 }
 
 function* testBottomRight(inspector, view) {
   yield assertPseudoElementRulesNumbers("#bottomright", inspector, view, {
     elementRulesNb: 4,
     firstLineRulesNb: 1,
     firstLetterRulesNb: 1,
     selectionRulesNb: 0
@@ -141,71 +146,46 @@ function* testBottomLeft(inspector, view
     elementRulesNb: 4,
     firstLineRulesNb: 1,
     firstLetterRulesNb: 1,
     selectionRulesNb: 0
   });
 }
 
 function* testParagraph(inspector, view) {
-  let {
-    rules,
-    element,
-    elementStyle
-  } = yield assertPseudoElementRulesNumbers("#bottomleft p", inspector, view, {
+  let {rules} = yield assertPseudoElementRulesNumbers("#bottomleft p", inspector, view, {
     elementRulesNb: 3,
     firstLineRulesNb: 1,
     firstLetterRulesNb: 1,
     selectionRulesNb: 1
   });
 
-  let gutters = assertGutters(view);
+  assertGutters(view);
 
   let elementFirstLineRule = rules.firstLineRules[0];
-  let elementFirstLineRuleView = [].filter.call(view.element.children[1].children, (e) => {
-    return e._ruleEditor && e._ruleEditor.rule === elementFirstLineRule;
-  })[0]._ruleEditor;
-
-  is
-  (
-    convertTextPropsToString(elementFirstLineRule.textProps),
-    "background: blue none repeat scroll 0% 0%",
-    "Paragraph first-line properties are correct"
-  );
+  is(convertTextPropsToString(elementFirstLineRule.textProps),
+     "background: blue none repeat scroll 0% 0%",
+     "Paragraph first-line properties are correct");
 
   let elementFirstLetterRule = rules.firstLetterRules[0];
-  let elementFirstLetterRuleView = [].filter.call(view.element.children[1].children, (e) => {
-    return e._ruleEditor && e._ruleEditor.rule === elementFirstLetterRule;
-  })[0]._ruleEditor;
-
-  is
-  (
-    convertTextPropsToString(elementFirstLetterRule.textProps),
-    "color: red; font-size: 130%",
-    "Paragraph first-letter properties are correct"
-  );
+  is(convertTextPropsToString(elementFirstLetterRule.textProps),
+     "color: red; font-size: 130%",
+     "Paragraph first-letter properties are correct");
 
   let elementSelectionRule = rules.selectionRules[0];
-  let elementSelectionRuleView = [].filter.call(view.element.children[1].children, (e) => {
-    return e._ruleEditor && e._ruleEditor.rule === elementSelectionRule;
-  })[0]._ruleEditor;
-
-  is
-  (
-    convertTextPropsToString(elementSelectionRule.textProps),
-    "color: white; background: black none repeat scroll 0% 0%",
-    "Paragraph first-letter properties are correct"
-  );
+  is(convertTextPropsToString(elementSelectionRule.textProps),
+     "color: white; background: black none repeat scroll 0% 0%",
+     "Paragraph first-letter properties are correct");
 }
 
 function* testBody(inspector, view) {
-  let {element, elementStyle} = yield testNode("body", inspector, view);
+  yield testNode("body", inspector, view);
 
-  let gutters = view.element.querySelectorAll(".theme-gutter");
-  is (gutters.length, 0, "There are no gutter headings");
+  let gutters = getGutters(view);
+  is(gutters.length, 0, "There are no gutter headings");
 }
 
 function convertTextPropsToString(textProps) {
   return textProps.map(t => t.name + ": " + t.value).join("; ");
 }
 
 function* testNode(selector, inspector, view) {
   let element = getNode(selector);
@@ -219,29 +199,38 @@ function* assertPseudoElementRulesNumber
 
   let rules = {
     elementRules: elementStyle.rules.filter(rule => !rule.pseudoElement),
     firstLineRules: elementStyle.rules.filter(rule => rule.pseudoElement === ":first-line"),
     firstLetterRules: elementStyle.rules.filter(rule => rule.pseudoElement === ":first-letter"),
     selectionRules: elementStyle.rules.filter(rule => rule.pseudoElement === ":-moz-selection")
   };
 
-  is(rules.elementRules.length, ruleNbs.elementRulesNb, selector +
-    " has the correct number of non pseudo element rules");
-  is(rules.firstLineRules.length, ruleNbs.firstLineRulesNb, selector +
-    " has the correct number of :first-line rules");
-  is(rules.firstLetterRules.length, ruleNbs.firstLetterRulesNb, selector +
-    " has the correct number of :first-letter rules");
-  is(rules.selectionRules.length, ruleNbs.selectionRulesNb, selector +
-    " has the correct number of :selection rules");
+  is(rules.elementRules.length, ruleNbs.elementRulesNb,
+     selector + " has the correct number of non pseudo element rules");
+  is(rules.firstLineRules.length, ruleNbs.firstLineRulesNb,
+     selector + " has the correct number of :first-line rules");
+  is(rules.firstLetterRules.length, ruleNbs.firstLetterRulesNb,
+     selector + " has the correct number of :first-letter rules");
+  is(rules.selectionRules.length, ruleNbs.selectionRulesNb,
+     selector + " has the correct number of :selection rules");
 
-  return {rules: rules, element: element, elementStyle: elementStyle};
+  return {rules, element, elementStyle};
+}
+
+function getGutters(view) {
+  return view.element.querySelectorAll(".theme-gutter");
 }
 
 function assertGutters(view) {
-  let gutters = view.element.querySelectorAll(".theme-gutter");
-  is (gutters.length, 3, "There are 3 gutter headings");
-  is (gutters[0].textContent, "Pseudo-elements", "Gutter heading is correct");
-  is (gutters[1].textContent, "This Element", "Gutter heading is correct");
-  is (gutters[2].textContent, "Inherited from body", "Gutter heading is correct");
+  let gutters = getGutters(view);
+
+  is(gutters.length, 3,
+     "There are 3 gutter headings");
+  is(gutters[0].textContent, "Pseudo-elements",
+     "Gutter heading is correct");
+  is(gutters[1].textContent, "This Element",
+     "Gutter heading is correct");
+  is(gutters[2].textContent, "Inherited from body",
+     "Gutter heading is correct");
 
   return gutters;
 }