Bug 1321791 - [Rule View] Add indicator if parts of compound (shorthand) CSS rule are overridden; r=gl
authorRahul Chaudhary <rahulch95@gmail.com>
Mon, 13 Feb 2017 09:04:07 -0500
changeset 372798 dc9764f81e1a14a187331a873300f181f92d7e88
parent 372797 5fbcbdd6849379c48ce869d4ed0e8083af9b8d23
child 372799 32242abfde5d17bcc1c7c82df0d395c1cc64f955
push id10863
push userjlorenzo@mozilla.com
push dateMon, 06 Mar 2017 23:02:23 +0000
treeherdermozilla-aurora@0931190cd725 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgl
bugs1321791
milestone54.0a1
Bug 1321791 - [Rule View] Add indicator if parts of compound (shorthand) CSS rule are overridden; r=gl MozReview-Commit-ID: 2ANP3pD7uuU
devtools/client/inspector/rules/test/browser.ini
devtools/client/inspector/rules/test/browser_rules_shorthand-overridden-lists.js
devtools/client/inspector/rules/views/text-property-editor.js
devtools/client/themes/rules.css
--- a/devtools/client/inspector/rules/test/browser.ini
+++ b/devtools/client/inspector/rules/test/browser.ini
@@ -210,16 +210,17 @@ subsuite = clipboard
 skip-if = (os == 'linux' && bits == 32 && debug) # bug 1328915, disable linux32 debug devtools for timeouts
 [browser_rules_selector-highlighter-on-navigate.js]
 [browser_rules_selector-highlighter_01.js]
 [browser_rules_selector-highlighter_02.js]
 [browser_rules_selector-highlighter_03.js]
 [browser_rules_selector-highlighter_04.js]
 [browser_rules_selector-highlighter_05.js]
 [browser_rules_selector_highlight.js]
+[browser_rules_shorthand-overridden-lists.js]
 [browser_rules_strict-search-filter-computed-list_01.js]
 [browser_rules_strict-search-filter_01.js]
 [browser_rules_strict-search-filter_02.js]
 [browser_rules_strict-search-filter_03.js]
 [browser_rules_style-editor-link.js]
 [browser_rules_urls-clickable.js]
 [browser_rules_user-agent-styles.js]
 [browser_rules_user-agent-styles-uneditable.js]
new file mode 100644
--- /dev/null
+++ b/devtools/client/inspector/rules/test/browser_rules_shorthand-overridden-lists.js
@@ -0,0 +1,70 @@
+/* vim: set ft=javascript ts=2 et sw=2 tw=80: */
+/* Any copyright is dedicated to the Public Domain.
+ http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+// Tests that the rule view shorthand overridden list works correctly,
+// can be shown and hidden correctly, and contain the right subproperties.
+
+var TEST_URI = `
+  <style type="text/css">
+    div {
+      margin: 0px 1px 2px 3px;
+      top: 0px;
+    }
+    #testid {
+        margin-left: 10px;
+        margin-right: 10px;
+    }
+  </style>
+  <div id="testid">Styled Node</div>
+`;
+
+add_task(function* () {
+  yield addTab("data:text/html;charset=utf-8," + encodeURIComponent(TEST_URI));
+  let {inspector, view} = yield openRuleView();
+  yield selectNode("#testid", inspector);
+  yield testComputedList(inspector, view);
+});
+
+function* testComputedList(inspector, view) {
+  let rule = getRuleViewRuleEditor(view, 2).rule;
+  let propEditor = rule.textProps[0].editor;
+  let expander = propEditor.expander;
+  let overriddenItems = propEditor.shorthandOverridden.children;
+  let propNames = [
+    "margin-right",
+    "margin-left"
+  ];
+
+  ok(!expander.hasAttribute("open"), "margin computed list is closed.");
+  ok(!propEditor.shorthandOverridden.hasAttribute("hidden"),
+      "The shorthandOverridden list should be open.");
+
+  is(overriddenItems.length, propNames.length,
+      "There should be 2 overridden shorthand value.");
+  for (let i = 0; i < propNames.length; i++) {
+    let overriddenItem = overriddenItems[i].querySelector(".ruleview-propertyname");
+    is(overriddenItem.textContent, propNames[i],
+        "The overridden item #" + i + " should be " + propNames[i]);
+  }
+
+  info("Opening the computed list of margin property.");
+  expander.click();
+  ok(expander.hasAttribute("open"), "margin computed list is open.");
+  ok(propEditor.shorthandOverridden.hasAttribute("hidden"),
+      "The shorthandOverridden list should be hidden.");
+
+  info("Closing the computed list of margin property.");
+  expander.click();
+  ok(!expander.hasAttribute("open"), "margin computed list is closed.");
+  ok(!propEditor.shorthandOverridden.hasAttribute("hidden"),
+      "The shorthandOverridden list should be open.");
+
+  for (let i = 0; i < propNames.length; i++) {
+    let overriddenItem = overriddenItems[i].querySelector(".ruleview-propertyname");
+    is(overriddenItem.textContent, propNames[i],
+        "The overridden item #" + i + " should still be " + propNames[i]);
+  }
+}
--- a/devtools/client/inspector/rules/views/text-property-editor.js
+++ b/devtools/client/inspector/rules/views/text-property-editor.js
@@ -192,16 +192,22 @@ TextPropertyEditor.prototype = {
     });
 
     // Holds the viewers for the computed properties.
     // will be populated in |_updateComputed|.
     this.computed = createChild(this.element, "ul", {
       class: "ruleview-computedlist",
     });
 
+    // Holds the viewers for the overridden shorthand properties.
+    // will be populated in |_updateShorthandOverridden|.
+    this.shorthandOverridden = createChild(this.element, "ul", {
+      class: "ruleview-overridden-items",
+    });
+
     // Only bind event handlers if the rule is editable.
     if (this.ruleEditor.isEditable) {
       this.enable.addEventListener("click", this._onEnableClicked, true);
 
       this.nameContainer.addEventListener("click", (event) => {
         // Clicks within the name shouldn't propagate any further.
         event.stopPropagation();
 
@@ -449,32 +455,33 @@ TextPropertyEditor.prototype = {
       }
 
       if (!elToClick) {
         elToClick = this.valueSpan;
       }
       elToClick.click();
     }
 
-    // Populate the computed styles.
+    // Populate the computed styles and shorthand overridden styles.
     this._updateComputed();
+    this._updateShorthandOverridden();
 
     // Update the rule property highlight.
     this.ruleView._updatePropertyHighlight(this);
   },
 
   _onStartEditing: function () {
     this.element.classList.remove("ruleview-overridden");
     this.filterProperty.hidden = true;
     this.enable.style.visibility = "hidden";
   },
 
   /**
    * Update the visibility of the enable checkbox, the warning indicator and
-   * the filter property, as well as the overriden state of the property.
+   * the filter property, as well as the overridden state of the property.
    */
   updatePropertyState: function () {
     if (this.prop.enabled) {
       this.enable.style.removeProperty("visibility");
       this.enable.setAttribute("checked", "");
     } else {
       this.enable.style.visibility = "visible";
       this.enable.removeAttribute("checked");
@@ -522,53 +529,94 @@ TextPropertyEditor.prototype = {
 
     for (let computed of this.prop.computed) {
       // Don't bother to duplicate information already
       // shown in the text property.
       if (computed.name === this.prop.name) {
         continue;
       }
 
-      let li = createChild(this.computed, "li", {
-        class: "ruleview-computed"
-      });
+      // Store the computed style element for easy access when highlighting
+      // styles
+      computed.element = this._createComputedListItem(this.computed, computed,
+        "ruleview-computed");
+    }
+  },
+
+  /**
+   * Update the indicator for overridden shorthand styles. The shorthand
+   * overridden styles themselves are populated on demand, when they
+   * become visible.
+   */
+  _updateShorthandOverridden: function () {
+    this.shorthandOverridden.innerHTML = "";
 
-      if (computed.overridden) {
-        li.classList.add("ruleview-overridden");
+    this._populatedShorthandOverridden = false;
+    this._populateShorthandOverridden();
+  },
+
+  /**
+   * Populate the list of overridden shorthand styles.
+   */
+  _populateShorthandOverridden: function () {
+    if (this._populatedShorthandOverridden || this.prop.overridden) {
+      return;
+    }
+    this._populatedShorthandOverridden = true;
+
+    for (let computed of this.prop.computed) {
+      // Don't display duplicate information or show properties
+      // that are completely overridden.
+      if (computed.name === this.prop.name || !computed.overridden) {
+        continue;
       }
 
-      createChild(li, "span", {
-        class: "ruleview-propertyname theme-fg-color5",
-        textContent: computed.name
-      });
-      appendText(li, ": ");
+      this._createComputedListItem(this.shorthandOverridden, computed,
+        "ruleview-overridden-item");
+    }
+  },
 
-      let outputParser = this.ruleView._outputParser;
-      let frag = outputParser.parseCssProperty(
-        computed.name, computed.value, {
-          colorSwatchClass: "ruleview-swatch ruleview-colorswatch",
-          urlClass: "theme-link",
-          baseURI: this.sheetHref
-        }
-      );
+  /**
+   * Creates and populates a list item with the computed CSS property.
+   */
+  _createComputedListItem: function (parentEl, computed, className) {
+    let li = createChild(parentEl, "li", {
+      class: className
+    });
+
+    if (computed.overridden) {
+      li.classList.add("ruleview-overridden");
+    }
 
-      // Store the computed property value that was parsed for output
-      computed.parsedValue = frag.textContent;
+    createChild(li, "span", {
+      class: "ruleview-propertyname theme-fg-color5",
+      textContent: computed.name
+    });
+    appendText(li, ": ");
 
-      createChild(li, "span", {
-        class: "ruleview-propertyvalue theme-fg-color1",
-        child: frag
-      });
+    let outputParser = this.ruleView._outputParser;
+    let frag = outputParser.parseCssProperty(
+      computed.name, computed.value, {
+        colorSwatchClass: "ruleview-swatch ruleview-colorswatch",
+        urlClass: "theme-link",
+        baseURI: this.sheetHref
+      }
+    );
 
-      appendText(li, ";");
+    // Store the computed property value that was parsed for output
+    computed.parsedValue = frag.textContent;
 
-      // Store the computed style element for easy access when highlighting
-      // styles
-      computed.element = li;
-    }
+    createChild(li, "span", {
+      class: "ruleview-propertyvalue theme-fg-color1",
+      child: frag
+    });
+
+    appendText(li, ";");
+
+    return li;
   },
 
   /**
    * Handles clicks on the disabled property.
    */
   _onEnableClicked: function (event) {
     let checked = this.enable.hasAttribute("checked");
     if (checked) {
@@ -588,19 +636,22 @@ TextPropertyEditor.prototype = {
    * expanded by manually by the user.
    */
   _onExpandClicked: function (event) {
     if (this.computed.hasAttribute("filter-open") ||
         this.computed.hasAttribute("user-open")) {
       this.expander.removeAttribute("open");
       this.computed.removeAttribute("filter-open");
       this.computed.removeAttribute("user-open");
+      this.shorthandOverridden.removeAttribute("hidden");
+      this._populateShorthandOverridden();
     } else {
       this.expander.setAttribute("open", "true");
       this.computed.setAttribute("user-open", "");
+      this.shorthandOverridden.setAttribute("hidden", "true");
       this._populateComputed();
     }
 
     event.stopPropagation();
   },
 
   /**
    * Expands the computed list when a computed property is matched by the style
--- a/devtools/client/themes/rules.css
+++ b/devtools/client/themes/rules.css
@@ -118,23 +118,25 @@
   padding-right: 5px;
 }
 
 .ruleview-propertyvaluecontainer a {
   cursor: pointer;
 }
 
 .ruleview-computedlist,
+.ruleview-overridden-items[hidden],
 .ruleview-overridden-rule-filter[hidden],
 .ruleview-warning[hidden] {
   display: none;
 }
 
 .ruleview-computedlist[user-open],
-.ruleview-computedlist[filter-open] {
+.ruleview-computedlist[filter-open],
+.ruleview-overridden-items {
   display: block;
 }
 
 .ruleview-rule-source {
   text-align: end;
   float: right;
   max-width: 100%;
 
@@ -357,16 +359,52 @@
   list-style: none;
   padding: 0;
 }
 
 .ruleview-computed {
   margin-inline-start: 35px;
 }
 
+.ruleview-overridden-items {
+  margin: 0px 0px 0px 5px;
+  list-style: none;
+  line-height: 1.5em;
+}
+
+.ruleview-overridden-item {
+  position: relative;
+}
+
+.ruleview-overridden-item::before {
+  position: absolute;
+  left: -15px;
+  top: 0px;
+  content: '';
+  display: block;
+  border-left: 1px solid var(--theme-highlight-gray);
+  height: 0.7em;
+  border-bottom: 1px solid var(--theme-highlight-gray);
+  width: 10px;
+}
+
+.ruleview-overridden-item::after {
+  position: absolute;
+  left: -15px;
+  bottom: -7px;
+  content: '';
+  display: block;
+  border-left: 1px solid var(--theme-highlight-gray);
+  height: 100%;
+}
+
+.ruleview-overridden-item:last-child:after {
+  display: none;
+}
+
 .ruleview-grid,
 .ruleview-swatch {
   cursor: pointer;
   border-radius: 50%;
   width: 1em;
   height: 1em;
   vertical-align: middle;
   /* align the swatch with its value */