Bug 1355351: Look for the frame for ::before and ::after pseudos. r=heycam draft
authorEmilio Cobos Álvarez <emilio@crisal.io>
Sun, 16 Apr 2017 22:59:29 +0200
changeset 568270 ec399ec869742b141d3648e4a0ce84ba4bb05635
parent 568269 8879dc3a6b637f1d0ba965f81658ca88c320dc40
child 568271 aa1b1b7f851eba2ba42d5f57f6aec6523534fc03
push id55809
push userbmo:emilio+bugs@crisal.io
push dateTue, 25 Apr 2017 22:14:00 +0000
reviewersheycam
bugs1355351
milestone55.0a1
Bug 1355351: Look for the frame for ::before and ::after pseudos. r=heycam MozReview-Commit-ID: 7Hm2IGsl323
devtools/client/inspector/boxmodel/test/browser_boxmodel_pseudo-element.js
devtools/client/inspector/computed/test/browser_computed_pseudo-element_01.js
layout/style/nsComputedDOMStyle.cpp
--- a/devtools/client/inspector/boxmodel/test/browser_boxmodel_pseudo-element.js
+++ b/devtools/client/inspector/boxmodel/test/browser_boxmodel_pseudo-element.js
@@ -1,15 +1,15 @@
 /* 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 the box model displays the right values for a pseduo-element.
+// Test that the box model displays the right values for a pseudo-element.
 
 const TEST_URI = `
   <style type='text/css'>
     div {
       box-sizing: border-box;
       display: block;
       float: left;
       line-height: 20px;
@@ -48,25 +48,25 @@ const res1 = [
     value: "32"
   },
   {
     selector: ".boxmodel-margin.boxmodel-top > span",
     value: 0
   },
   {
     selector: ".boxmodel-margin.boxmodel-left > span",
-    value: "auto"
+    value: 0
   },
   {
     selector: ".boxmodel-margin.boxmodel-bottom > span",
     value: 6
   },
   {
     selector: ".boxmodel-margin.boxmodel-right > span",
-    value: "auto"
+    value: 0
   },
   {
     selector: ".boxmodel-padding.boxmodel-top > span",
     value: 0
   },
   {
     selector: ".boxmodel-padding.boxmodel-left > span",
     value: 0
--- a/devtools/client/inspector/computed/test/browser_computed_pseudo-element_01.js
+++ b/devtools/client/inspector/computed/test/browser_computed_pseudo-element_01.js
@@ -28,12 +28,12 @@ function* testTopLeft(inspector, view) {
   let top = getComputedViewPropertyValue(view, "top");
   is(top, "0px", "The computed view shows the correct top");
   let left = getComputedViewPropertyValue(view, "left");
   is(left, "0px", "The computed view shows the correct left");
 
   let afterElement = children.nodes[children.nodes.length - 1];
   yield selectNode(afterElement, inspector);
   top = getComputedViewPropertyValue(view, "top");
-  is(top, "50%", "The computed view shows the correct top");
+  is(top, "96px", "The computed view shows the correct top");
   left = getComputedViewPropertyValue(view, "left");
-  is(left, "50%", "The computed view shows the correct left");
+  is(left, "96px", "The computed view shows the correct left");
 }
--- a/layout/style/nsComputedDOMStyle.cpp
+++ b/layout/style/nsComputedDOMStyle.cpp
@@ -710,16 +710,44 @@ nsComputedDOMStyle::SetResolvedStyleCont
 
 void
 nsComputedDOMStyle::SetFrameStyleContext(nsStyleContext* aContext)
 {
   ClearStyleContext();
   mStyleContext = aContext;
 }
 
+/**
+ * The following function checks whether we need to explicitly resolve the style
+ * again, even though we have a style context coming from the frame.
+ *
+ * This basically checks whether the style is or may be under a ::first-line or
+ * ::first-letter frame, in which case we can't return the frame style, and we
+ * need to resolve it. See bug 505515.
+ */
+static bool
+MustReresolveStyle(const nsStyleContext* aContext)
+{
+  MOZ_ASSERT(aContext);
+
+  if (aContext->HasPseudoElementData()) {
+    if (!aContext->GetPseudo() ||
+        aContext->StyleSource().IsServoComputedValues()) {
+      // TODO(emilio): When ::first-line is supported in Servo, we may want to
+      // fix this to avoid re-resolving pseudo-element styles.
+      return true;
+    }
+
+    return aContext->GetParent() &&
+           aContext->GetParent()->HasPseudoElementData();
+  }
+
+  return false;
+}
+
 void
 nsComputedDOMStyle::UpdateCurrentStyleSources(bool aNeedsLayoutFlush)
 {
   nsCOMPtr<nsIDocument> document = do_QueryReferent(mDocumentWeak);
   if (!document) {
     ClearStyleContext();
     return;
   }
@@ -753,18 +781,31 @@ nsComputedDOMStyle::UpdateCurrentStyleSo
     // We've processed some restyles, so the cached style context might
     // be out of date.
     mStyleContext = nullptr;
   }
 
   // XXX the !mContent->IsHTMLElement(nsGkAtoms::area)
   // check is needed due to bug 135040 (to avoid using
   // mPrimaryFrame). Remove it once that's fixed.
-  if (!mPseudo && !mContent->IsHTMLElement(nsGkAtoms::area)) {
-    mOuterFrame = mContent->GetPrimaryFrame();
+  if (!mContent->IsHTMLElement(nsGkAtoms::area)) {
+    mOuterFrame = nullptr;
+
+    if (!mPseudo) {
+      mOuterFrame = mContent->GetPrimaryFrame();
+    } else if (mPseudo == nsCSSPseudoElements::before ||
+               mPseudo == nsCSSPseudoElements::after) {
+      nsIAtom* property = mPseudo == nsCSSPseudoElements::before
+                            ? nsGkAtoms::beforePseudoProperty
+                            : nsGkAtoms::afterPseudoProperty;
+
+      auto* pseudo = static_cast<Element*>(mContent->GetProperty(property));
+      mOuterFrame = pseudo ? pseudo->GetPrimaryFrame() : nullptr;
+    }
+
     mInnerFrame = mOuterFrame;
     if (mOuterFrame) {
       nsIAtom* type = mOuterFrame->GetType();
       if (type == nsGkAtoms::tableWrapperFrame) {
         // If the frame is a table wrapper frame then we should get the style
         // from the inner table frame.
         mInnerFrame = mOuterFrame->PrincipalChildList().FirstChild();
         NS_ASSERTION(mInnerFrame, "table wrapper must have an inner");
@@ -773,17 +814,17 @@ nsComputedDOMStyle::UpdateCurrentStyleSo
                      "the inner table");
       }
 
       SetFrameStyleContext(mInnerFrame->StyleContext());
       NS_ASSERTION(mStyleContext, "Frame without style context?");
     }
   }
 
-  if (!mStyleContext || mStyleContext->HasPseudoElementData()) {
+  if (!mStyleContext || MustReresolveStyle(mStyleContext)) {
 #ifdef DEBUG
     if (mStyleContext && mStyleContext->StyleSource().IsGeckoRuleNodeOrNull()) {
       // We want to check that going through this path because of
       // HasPseudoElementData is rare, because it slows us down a good
       // bit.  So check that we're really inside something associated
       // with a pseudo-element that contains elements.  (We also allow
       // the element to be NAC, just in case some chrome JS calls
       // getComputedStyle on a NAC-implemented pseudo.)