Bug 1388877. Fix insertions under a ::first-line in stylo. r=heycam

MozReview-Commit-ID: CDolJpTtGki

layout/base/ServoRestyleManager.cpp

@@ -1436,18 +1436,17 @@ nsresult
 ServoRestyleManager::ReparentStyleContext(nsIFrame* aFrame)
 {
   // This is only called when moving frames in or out of the first-line
-  // pseudo-element (or one of its inline descendants).  So aFrame's ancestors
-  // must all be inline frames up until we find a first-line frame.  Note that
-  // the first-line frame may not actually be the one that corresponds to
-  // ::first-line; when we're moving _out_ of the first-line it will be one of
-  // the continuations instead.
+  // pseudo-element (or one of its descendants).  We can't say much about
+  // aFrame's ancestors, unfortunately (e.g. during a dynamic insert into
+  // something inside an inline-block on the first line the ancestors could be
+  // totally arbitrary), but we will definitely find a line frame on the
+  // ancestor chain.  Note that the lineframe may not actually be the one that
+  // corresponds to ::first-line; when we're moving _out_ of the ::first-line it
+  // will be one of the continuations instead.
 #ifdef DEBUG
   {
     nsIFrame* f = aFrame->GetParent();
     while (f && !f->IsLineFrame()) {
-      MOZ_ASSERT(f->IsInlineFrame(),
-                 "Must only have inline frames between us and the first-line "
-                 "frame");
       f = f->GetParent();
     }
     MOZ_ASSERT(f, "Must have found a first-line frame");

layout/base/nsCSSFrameConstructor.cpp

@@ -7895,11 +7895,23 @@ nsCSSFrameConstructor::ContentAppended(nsIContent* aContainer,
     PullOutCaptionFrames(frameItems, captionItems);
   }
 
+  bool dealtWithFirstLine = false;
   if (haveFirstLineStyle && parentFrame == containingBlock) {
     // It's possible that some of the new frames go into a
     // first-line frame. Look at them and see...
     AppendFirstLineFrames(state, containingBlock->GetContent(),
                           containingBlock, frameItems);
+    // That moved things into line frames as needed, reparenting their
+    // styles.  Nothing else needs to be done.
+    dealtWithFirstLine = true;
+  }
+
+  if (!dealtWithFirstLine &&
+      parentFrame->StyleContext()->HasPseudoElementData()) {
+    // parentFrame might be inside a ::first-line frame.  Check whether it is,
+    // and if so fix up our styles.
+    CheckForFirstLineInsertion(parentFrame, frameItems);
+    CheckForFirstLineInsertion(parentFrame, captionItems);
   }
 
   // Notify the parent frame passing it the list of new frames
@@ -8495,6 +8507,7 @@ nsCSSFrameConstructor::ContentRangeInserted(nsIContent* aContainer,
     prevSibling = ::FindAppendPrevSibling(insertion.mParentFrame, appendAfterFrame);
   }
 
+  bool dealtWithFirstLine = false;
   if (haveFirstLineStyle && insertion.mParentFrame == containingBlock) {
     // It's possible that the new frame goes into a first-line
     // frame. Look at it and see...
@@ -8502,17 +8515,28 @@ nsCSSFrameConstructor::ContentRangeInserted(nsIContent* aContainer,
       // Use append logic when appending
       AppendFirstLineFrames(state, containingBlock->GetContent(),
                             containingBlock, frameItems);
+      // That moved things into line frames as needed, reparenting their
+      // styles.  Nothing else needs to be done to handle ::first-line.
+      dealtWithFirstLine = true;
     }
     else {
       // Use more complicated insert logic when inserting
       // XXXbz this method is a no-op, so it's easy for the args being passed
       // here to make no sense without anyone noticing...  If it ever stops
       // being a no-op, vet them carefully!
+      // XXXbz Can this code even get hit?  I'd think/hope not, since any
+      // insert would go into an existing lineframe if we have them..
       InsertFirstLineFrames(state, container, containingBlock, &insertion.mParentFrame,
                             prevSibling, frameItems);
     }
   }
 
+  if (!dealtWithFirstLine &&
+      insertion.mParentFrame->StyleContext()->HasPseudoElementData()) {
+    CheckForFirstLineInsertion(insertion.mParentFrame, frameItems);
+    CheckForFirstLineInsertion(insertion.mParentFrame, captionItems);
+  }
+
   // We might have captions; put them into the caption list of the
   // table wrapper frame.
   if (captionItems.NotEmpty()) {
@@ -11708,6 +11732,56 @@ nsCSSFrameConstructor::InsertFirstLineFrames(
 #endif
 }
 
+void
+nsCSSFrameConstructor::CheckForFirstLineInsertion(nsIFrame* aParentFrame,
+                                                  nsFrameItems& aFrameItems)
+{
+  MOZ_ASSERT(aParentFrame->StyleContext()->HasPseudoElementData(),
+             "Why were we called?");
+
+  if (aFrameItems.IsEmpty()) {
+    // Happens often enough, with the caption stuff.  No need to do the ancestor
+    // walk here.
+    return;
+  }
+
+  class RestyleManager* restyleManager = RestyleManager();
+  if (!restyleManager->IsServo()) {
+    // Gecko's style resolution is frame-based, so already has the right styles
+    // even in the ::first-line case.
+    return;
+  }
+
+  // Check whether there's a ::first-line on the path up from aParentFrame.
+  // Note that we can't stop until we've run out of ancestors with
+  // pseudo-element data, because the first-letter might be somewhere way up the
+  // tree; in particular it might be past our containing block.
+  nsIFrame* ancestor = aParentFrame;
+  while (ancestor) {
+    if (!ancestor->StyleContext()->HasPseudoElementData()) {
+      // We know we won't find a ::first-line now.
+      return;
+    }
+
+    if (!ancestor->IsLineFrame()) {
+      ancestor = ancestor->GetParent();
+      continue;
+    }
+
+    if (!ancestor->StyleContext()->IsPseudoElement()) {
+      // This is a continuation lineframe, not the first line; no need to do
+      // anything to the styles.
+      return;
+    }
+
+    // Fix up the styles of aFrameItems for ::first-line.
+    for (nsIFrame* f : aFrameItems) {
+      restyleManager->ReparentStyleContext(f);
+    }
+    return;
+  }
+}
+
 //----------------------------------------------------------------------
 
 // First-letter support

layout/base/nsCSSFrameConstructor.h

@@ -2051,6 +2051,18 @@ private:
                              nsIFrame*                aPrevSibling,
                              nsFrameItems&            aFrameItems);
 
+  /**
+   * When aFrameItems is being inserted into aParentFrame, and aParentFrame has
+   * pseudo-element-affected styles, it's possible that we're inserting under a
+   * ::first-line frame.  In that case, with servo's style system, the styles we
+   * resolved for aFrameItems are wrong (they don't take ::first-line into
+   * account), and we should fix them up, which is what this method does.
+   *
+   * This method does not mutate aFrameItems.
+   */
+  void CheckForFirstLineInsertion(nsIFrame* aParentFrame,
+                                  nsFrameItems& aFrameItems);
+
   /**
    * Find the right frame to use for aContent when looking for sibling
    * frames for aTargetContent.  If aPrevSibling is true, this

layout/reftests/bugs/reftest.list

@@ -1383,7 +1383,7 @@ pref(dom.use_xbl_scopes_for_remote_xul,true) == 495385-2f.xhtml 495385-2-ref.htm
 == 495385-2i.html 495385-2-ref.html
 == 495385-3.html 495385-3-ref.html
 == 495385-4.html 495385-4-ref.html
-fails-if(styloVsGecko||stylo) == 495385-5.html 495385-5-ref.html
+== 495385-5.html 495385-5-ref.html
 == 496032-1.html 496032-1-ref.html
 == 496840-1.html 496840-1-ref.html
 fuzzy-if(skiaContent,1,17000) == 498228-1.xul 498228-1-ref.xul

layout/reftests/first-line/caption-insert-in-first-line-1.html

@@ -0,0 +1,20 @@
+<!DOCTYPE html>
+<style>
+  div { color: red; }
+  div::first-line { color: green }
+  #table { display: inline-table; }
+  #caption { display: table-caption; }
+  #tbody { display: table-row-group; }
+</style>
+<div id="x">
+  <span id="table">
+    <span id="tbody">be green</span>
+  </span>
+</div>
+<script>
+  x.offsetWidth;
+  var caption = document.createElement("span");
+  caption.id = "caption";
+  caption.textContent = "This should";
+  table.appendChild(caption);
+</script>

layout/reftests/first-line/caption-insert-in-first-line-2.html

@@ -0,0 +1,20 @@
+<!DOCTYPE html>
+<style>
+  div { color: red; }
+  div::first-line { color: green }
+  #table { display: inline-table; }
+  #caption { display: table-caption; }
+  #tbody { display: table-row-group; }
+</style>
+<div id="x">
+  <span id="table">
+    <span id="tbody">be green</span>
+  </span>
+</div>
+<script>
+  x.offsetWidth;
+  var caption = document.createElement("span");
+  caption.id = "caption";
+  caption.textContent = "This should";
+  table.insertBefore(caption, tbody);
+</script>

layout/reftests/first-line/caption-insert-in-first-line-ref.html

@@ -0,0 +1,13 @@
+<!DOCTYPE html>
+<style>
+  div { color: green; }
+  #table { display: inline-table; }
+  #caption { display: table-caption; }
+  #tbody { display: table-row-group; }
+</style>
+<div>
+  <span id="table">
+    <span id="caption">This should</span>
+    <span id="tbody">be green</span>
+  </span>
+</div>

layout/reftests/first-line/insertion-in-first-line-1.html

@@ -0,0 +1,10 @@
+<!DOCTYPE html>
+<style>
+  div { color: red; }
+  div::first-line { color: green }
+</style>
+<div id="x"></div>
+<script>
+  x.offsetWidth;
+  x.appendChild(document.createTextNode('This should be green'));
+</script>

layout/reftests/first-line/insertion-in-first-line-10.html

@@ -0,0 +1,13 @@
+<!DOCTYPE html>
+<style>
+  div { color: red; }
+  div::first-line { color: green }
+  #y { display: inline-block; }
+</style>
+<div id="x"><span id="y">This should be </span></div>
+<script>
+  x.offsetWidth;
+  var span = document.createElement('span');
+  span.textContent = 'green';
+  y.appendChild(span);
+</script>

layout/reftests/first-line/insertion-in-first-line-11.html

@@ -0,0 +1,11 @@
+<!DOCTYPE html>
+<style>
+  div { color: red; }
+  div::first-line { color: green }
+  #y { display: inline-block; }
+</style>
+<div id="x"><span id="y">green</span></div>
+<script>
+  x.offsetWidth;
+  y.insertBefore(document.createTextNode('This should be '), y.firstChild);
+</script>

layout/reftests/first-line/insertion-in-first-line-12.html

@@ -0,0 +1,13 @@
+<!DOCTYPE html>
+<style>
+  div { color: red; }
+  div::first-line { color: green }
+  #y { display: inline-block; }
+</style>
+<div id="x"><span id="y">green</span></div>
+<script>
+  x.offsetWidth;
+  var span = document.createElement('span');
+  span.textContent = 'This should be ';
+  y.insertBefore(span, y.firstChild);
+</script>

layout/reftests/first-line/insertion-in-first-line-2.html

@@ -0,0 +1,12 @@
+<!DOCTYPE html>
+<style>
+  div { color: red; }
+  div::first-line { color: green }
+</style>
+<div id="x"></div>
+<script>
+  x.offsetWidth;
+  var span = document.createElement('span');
+  span.textContent = 'This should be green';
+  x.appendChild(span);
+</script>

layout/reftests/first-line/insertion-in-first-line-3.html

@@ -0,0 +1,10 @@
+<!DOCTYPE html>
+<style>
+  div { color: red; }
+  div::first-line { color: green }
+</style>
+<div id="x"><span id="y">green</span></div>
+<script>
+  x.offsetWidth;
+  x.insertBefore(document.createTextNode('This should be '), y);
+</script>

layout/reftests/first-line/insertion-in-first-line-4.html

@@ -0,0 +1,12 @@
+<!DOCTYPE html>
+<style>
+  div { color: red; }
+  div::first-line { color: green }
+</style>
+<div id="x"><span id="y">green</span></div>
+<script>
+  x.offsetWidth;
+  var span = document.createElement('span');
+  span.textContent = 'This should be ';
+  x.insertBefore(span, y);
+</script>

layout/reftests/first-line/insertion-in-first-line-5.html

@@ -0,0 +1,10 @@
+<!DOCTYPE html>
+<style>
+  div { color: red; }
+  div::first-line { color: green }
+</style>
+<div id="x"><span id="y">This should be </span></div>
+<script>
+  x.offsetWidth;
+  y.appendChild(document.createTextNode('green'));
+</script>

layout/reftests/first-line/insertion-in-first-line-6.html

@@ -0,0 +1,12 @@
+<!DOCTYPE html>
+<style>
+  div { color: red; }
+  div::first-line { color: green }
+</style>
+<div id="x"><span id="y">This should be </span></div>
+<script>
+  x.offsetWidth;
+  var span = document.createElement('span');
+  span.textContent = 'green';
+  y.appendChild(span);
+</script>

layout/reftests/first-line/insertion-in-first-line-7.html

@@ -0,0 +1,10 @@
+<!DOCTYPE html>
+<style>
+  div { color: red; }
+  div::first-line { color: green }
+</style>
+<div id="x"><span id="y">green</span></div>
+<script>
+  x.offsetWidth;
+  y.insertBefore(document.createTextNode('This should be '), y.firstChild);
+</script>

layout/reftests/first-line/insertion-in-first-line-8.html

@@ -0,0 +1,12 @@
+<!DOCTYPE html>
+<style>
+  div { color: red; }
+  div::first-line { color: green }
+</style>
+<div id="x"><span id="y">green</span></div>
+<script>
+  x.offsetWidth;
+  var span = document.createElement('span');
+  span.textContent = 'This should be ';
+  y.insertBefore(span, y.firstChild);
+</script>

layout/reftests/first-line/insertion-in-first-line-9.html

@@ -0,0 +1,11 @@
+<!DOCTYPE html>
+<style>
+  div { color: red; }
+  div::first-line { color: green }
+  #y { display: inline-block; }
+</style>
+<div id="x"><span id="y">This should be </span></div>
+<script>
+  x.offsetWidth;
+  y.appendChild(document.createTextNode('green'));
+</script>

layout/reftests/first-line/insertion-in-first-line-ref.html

@@ -0,0 +1,5 @@
+<!DOCTYPE html>
+<style>
+  div { color: green; }
+</style>
+<div>This should be green</div>

layout/reftests/first-line/reftest.list

@@ -39,3 +39,18 @@ fuzzy-if(OSX==1010,1,2) == font-styles-nooverflow.html font-styles-ref.html
 fails-if(!stylo) == ib-split-1.html ib-split-1-ref.html
 
 == first-line-in-columnset-1.html first-line-in-columnset-1-ref.html
+
+== insertion-in-first-line-1.html insertion-in-first-line-ref.html
+== insertion-in-first-line-2.html insertion-in-first-line-ref.html
+== insertion-in-first-line-3.html insertion-in-first-line-ref.html
+== insertion-in-first-line-4.html insertion-in-first-line-ref.html
+== insertion-in-first-line-5.html insertion-in-first-line-ref.html
+== insertion-in-first-line-6.html insertion-in-first-line-ref.html
+== insertion-in-first-line-7.html insertion-in-first-line-ref.html
+== insertion-in-first-line-8.html insertion-in-first-line-ref.html
+== insertion-in-first-line-9.html insertion-in-first-line-ref.html
+== insertion-in-first-line-10.html insertion-in-first-line-ref.html
+== insertion-in-first-line-11.html insertion-in-first-line-ref.html
+== insertion-in-first-line-12.html insertion-in-first-line-ref.html
+== caption-insert-in-first-line-1.html caption-insert-in-first-line-ref.html
+== caption-insert-in-first-line-2.html caption-insert-in-first-line-ref.html