Bug 1350499 - exclude styles for pseudo-els when building boxmodel widget;r=gl draft
authorJulian Descottes <jdescottes@mozilla.com>
Wed, 29 Mar 2017 15:41:42 +0200
changeset 553746 8cceb22ad3a7326f72a3a112a904a741f2017f45
parent 553172 6ea713ccc9abea93126423fefb855d0e051c95e2
child 622168 a02814f74e48fc552ec59995e8213ce6e85402d4
push id51749
push userjdescottes@mozilla.com
push dateThu, 30 Mar 2017 15:37:26 +0000
reviewersgl
bugs1350499
milestone55.0a1
Bug 1350499 - exclude styles for pseudo-els when building boxmodel widget;r=gl MozReview-Commit-ID: GSoqlJ97KT7
devtools/client/inspector/boxmodel/box-model.js
devtools/client/inspector/boxmodel/test/browser.ini
devtools/client/inspector/boxmodel/test/browser_boxmodel_editablemodel_pseudo.js
devtools/server/actors/styles.js
devtools/shared/specs/styles.js
--- a/devtools/client/inspector/boxmodel/box-model.js
+++ b/devtools/client/inspector/boxmodel/box-model.js
@@ -153,17 +153,20 @@ BoxModel.prototype = {
           this.inspector.selection.isElementNode())) {
         return null;
       }
 
       let node = this.inspector.selection.nodeFront;
       let layout = yield this.inspector.pageStyle.getLayout(node, {
         autoMargins: true,
       });
-      let styleEntries = yield this.inspector.pageStyle.getApplied(node, {});
+      let styleEntries = yield this.inspector.pageStyle.getApplied(node, {
+        // We don't need styles applied to pseudo elements of the current node.
+        skipPseudo: true
+      });
       this.elementRules = styleEntries.map(e => e.rule);
 
       // Update the layout properties with whether or not the element's position is
       // editable with the geometry editor.
       let isPositionEditable = yield this.inspector.pageStyle.isPositionEditable(node);
       layout = Object.assign({}, layout, {
         isPositionEditable,
       });
--- a/devtools/client/inspector/boxmodel/test/browser.ini
+++ b/devtools/client/inspector/boxmodel/test/browser.ini
@@ -13,16 +13,17 @@ support-files =
   !/devtools/client/shared/test/test-actor-registry.js
 
 [browser_boxmodel.js]
 [browser_boxmodel_editablemodel.js]
 # [browser_boxmodel_editablemodel_allproperties.js]
 # Disabled for too many intermittent failures (bug 1009322)
 [browser_boxmodel_editablemodel_bluronclick.js]
 [browser_boxmodel_editablemodel_border.js]
+[browser_boxmodel_editablemodel_pseudo.js]
 [browser_boxmodel_editablemodel_stylerules.js]
 [browser_boxmodel_guides.js]
 [browser_boxmodel_navigation.js]
 skip-if = true # Bug 1336198
 [browser_boxmodel_offsetparent.js]
 [browser_boxmodel_positions.js]
 [browser_boxmodel_properties.js]
 [browser_boxmodel_pseudo-element.js]
new file mode 100644
--- /dev/null
+++ b/devtools/client/inspector/boxmodel/test/browser_boxmodel_editablemodel_pseudo.js
@@ -0,0 +1,57 @@
+/* vim: set ts=2 et sw=2 tw=80: */
+/* Any copyright is dedicated to the Public Domain.
+ http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+// Test that pseudo elements have no side effect on the box model widget for their
+// container. See bug 1350499.
+
+const TEST_URI =
+  `<style>
+    .test::before {
+      content: 'before';
+      margin-top: 5px;
+      padding-top: 5px;
+      width: 5px;
+    }
+  </style>
+  <div style='width:200px;'>
+    <div class=test></div>
+  </div>`;
+
+add_task(function* () {
+  yield addTab("data:text/html," + encodeURIComponent(TEST_URI));
+  let {inspector, view, testActor} = yield openBoxModelView();
+
+  yield selectNode(".test", inspector);
+
+  // No margin-top defined.
+  info("Test that margins are not impacted by a pseudo element");
+  is((yield getStyle(testActor, ".test", "margin-top")), "", "margin-top is correct");
+  yield checkValueInBoxModel(".boxmodel-margin.boxmodel-top", "0", view.document);
+
+  // No padding-top defined.
+  info("Test that paddings are not impacted by a pseudo element");
+  is((yield getStyle(testActor, ".test", "padding-top")), "", "padding-top is correct");
+  yield checkValueInBoxModel(".boxmodel-padding.boxmodel-top", "0", view.document);
+
+  // Width should be driven by the parent div.
+  info("Test that dimensions are not impacted by a pseudo element");
+  is((yield getStyle(testActor, ".test", "width")), "", "width is correct");
+  yield checkValueInBoxModel(".boxmodel-content.boxmodel-width", "200", view.document);
+});
+
+function* checkValueInBoxModel(selector, expectedValue, doc) {
+  let span = doc.querySelector(selector + " > span");
+  is(span.textContent, expectedValue, "Should have the right value in the box model.");
+
+  EventUtils.synthesizeMouseAtCenter(span, {}, doc.defaultView);
+  let editor = doc.querySelector(".styleinspector-propertyeditor");
+  ok(editor, "Should have opened the editor.");
+  is(editor.value, expectedValue, "Should have the right value in the editor.");
+
+  let onBlur = once(editor, "blur");
+  EventUtils.synthesizeKey("VK_RETURN", {}, doc.defaultView);
+  yield onBlur;
+}
--- a/devtools/server/actors/styles.js
+++ b/devtools/server/actors/styles.js
@@ -432,16 +432,17 @@ var PageStyleActor = protocol.ActorClass
    * @param object options
    *   `filter`: A string filter that affects the "matched" handling.
    *     'user': Include properties from user style sheets.
    *     'ua': Include properties from user and user-agent sheets.
    *     Default value is 'ua'
    *   `inherited`: Include styles inherited from parent nodes.
    *   `matchedSelectors`: Include an array of specific selectors that
    *     caused this rule to match its node.
+   *   `skipPseudo`: Exclude styles applied to pseudo elements of the provided node.
    */
   getApplied: Task.async(function* (node, options) {
     if (!node) {
       return {entries: [], rules: [], sheets: []};
     }
 
     this.cssLogic.highlight(node.rawNode);
     let entries = [];
@@ -532,17 +533,17 @@ var PageStyleActor = protocol.ActorClass
           // ::before/::after, and in this case we want to tell the
           // view that it belongs to the element (which is a
           // _moz_generated_content native anonymous element).
           oneRule.pseudoElement = null;
           rules.push(oneRule);
         });
 
     // Now any pseudos.
-    if (showElementStyles) {
+    if (showElementStyles && !options.skipPseudo) {
       for (let readPseudo of PSEUDO_ELEMENTS) {
         this._getElementRules(bindingElement, readPseudo, inherited, options)
             .forEach(oneRule => {
               rules.push(oneRule);
             });
       }
     }
 
@@ -631,16 +632,17 @@ var PageStyleActor = protocol.ActorClass
    * @param object options
    *   `filter`: A string filter that affects the "matched" handling.
    *     'user': Include properties from user style sheets.
    *     'ua': Include properties from user and user-agent sheets.
    *     Default value is 'ua'
    *   `inherited`: Include styles inherited from parent nodes.
    *   `matchedSelectors`: Include an array of specific selectors that
    *     caused this rule to match its node.
+   *   `skipPseudo`: Exclude styles applied to pseudo elements of the provided node.
    * @param array entries
    *   List of appliedstyle objects that lists the rules that apply to the
    *   node. If adding a new rule to the stylesheet, only the new rule entry
    *   is provided and only the style properties that apply to the new
    *   rule is fetched.
    * @returns Object containing the list of rule entries, rule actors and
    *   stylesheet actors that applies to the given node and its associated
    *   rules.
--- a/devtools/shared/specs/styles.js
+++ b/devtools/shared/specs/styles.js
@@ -129,16 +129,17 @@ const pageStyleSpec = generateActorSpec(
         matched: "array:matchedselector"
       }))
     },
     getApplied: {
       request: {
         node: Arg(0, "domnode"),
         inherited: Option(1, "boolean"),
         matchedSelectors: Option(1, "boolean"),
+        skipPseudo: Option(1, "boolean"),
         filter: Option(1, "string")
       },
       response: RetVal("appliedStylesReturn")
     },
     isPositionEditable: {
       request: { node: Arg(0, "domnode")},
       response: { value: RetVal("boolean") }
     },