Bug 1385656. Fix the interaction of RecoverLetterFrames and ::first-line. r?emilio draft
authorBoris Zbarsky <bzbarsky@mit.edu>
Thu, 10 Aug 2017 18:59:06 -0400
changeset 644437 ccd2e9be1ae7035792a4b2a417f1ad09d6215bfa
parent 644436 7f5df7afc2cba9f61e2370521649703bee13dfb3
child 725604 b70668836a53d71c2d04680796438ca1fa6b0a92
push id73434
push userbzbarsky@mozilla.com
push dateThu, 10 Aug 2017 22:59:22 +0000
reviewersemilio
bugs1385656
milestone57.0a1
Bug 1385656. Fix the interaction of RecoverLetterFrames and ::first-line. r?emilio MozReview-Commit-ID: BUt5FDI0IV1
layout/base/nsCSSFrameConstructor.cpp
layout/base/nsCSSFrameConstructor.h
layout/reftests/first-letter/1385656-ref.html
layout/reftests/first-letter/1385656.html
layout/reftests/first-letter/reftest.list
--- a/layout/base/nsCSSFrameConstructor.cpp
+++ b/layout/base/nsCSSFrameConstructor.cpp
@@ -7907,17 +7907,17 @@ nsCSSFrameConstructor::ContentAppended(n
   }
 
   if (frameItems.NotEmpty()) { // append the in-flow kids
     AppendFramesToParent(state, parentFrame, frameItems, prevSibling);
   }
 
   // Recover first-letter frames
   if (haveFirstLetterStyle) {
-    RecoverLetterFrames(containingBlock);
+    RecoverLetterFrames(containingBlock, haveFirstLineStyle);
   }
 
 #ifdef DEBUG
   if (gReallyNoisyContentUpdates) {
     printf("nsCSSFrameConstructor::ContentAppended: resulting frame model:\n");
     parentFrame->List(stdout, 0);
   }
 #endif
@@ -8340,17 +8340,18 @@ nsCSSFrameConstructor::ContentRangeInser
       // and creating frames.  We need to reget our prevsibling, parent frame,
       // etc.
       prevSibling = GetInsertionPrevSibling(&insertion, aStartChild, &isAppend,
                                             &isRangeInsertSafe);
 
       // Need check whether a range insert is still safe.
       if (!isSingleInsert && !isRangeInsertSafe) {
         // Need to recover the letter frames first.
-        RecoverLetterFrames(state.mFloatedItems.containingBlock);
+        RecoverLetterFrames(state.mFloatedItems.containingBlock,
+                            haveFirstLineStyle);
 
         // must fall back to a single ContertInserted for each child in the range
         LAYOUT_PHASE_TEMP_EXIT();
         IssueSingleInsertNofications(aContainer, aStartChild, aEndChild,
                                      aAllowLazyConstruction, aForReconstruction);
         LAYOUT_PHASE_TEMP_REENTER();
         return;
       }
@@ -8571,17 +8572,18 @@ nsCSSFrameConstructor::ContentRangeInser
     } else {
       InsertFrames(insertion.mParentFrame, kPrincipalList, prevSibling, frameItems);
     }
   }
 
   if (haveFirstLetterStyle) {
     // Recover the letter frames for the containing block when
     // it has first-letter style.
-    RecoverLetterFrames(state.mFloatedItems.containingBlock);
+    RecoverLetterFrames(state.mFloatedItems.containingBlock,
+                        haveFirstLineStyle);
   }
 
 #ifdef DEBUG
   if (gReallyNoisyContentUpdates && insertion.mParentFrame) {
     printf("nsCSSFrameConstructor::ContentRangeInserted: resulting frame model:\n");
     insertion.mParentFrame->List(stdout, 0);
   }
 #endif
@@ -8905,17 +8907,19 @@ nsCSSFrameConstructor::ContentRemoved(ns
       mRootElementFrame = nullptr;
       mRootElementStyleFrame = nullptr;
       mDocElementContainingBlock = nullptr;
       mPageSequenceFrame = nullptr;
       mHasRootAbsPosContainingBlock = false;
     }
 
     if (haveFLS && mRootElementFrame) {
-      RecoverLetterFrames(containingBlock);
+      RecoverLetterFrames(containingBlock,
+                          ShouldHaveFirstLineStyle(containingBlock->GetContent(),
+                                                   containingBlock->StyleContext()));
     }
 
     // If we're just reconstructing frames for the element, then the
     // following ContentInserted notification on the element will
     // take care of fixing up any adjacent text nodes.  We don't need
     // to do this if the table parent type of our parent type is not
     // eTypeBlock, though, because in that case the whitespace isn't
     // being suppressed due to us anyway.
@@ -9077,17 +9081,19 @@ nsCSSFrameConstructor::CharacterDataChan
         frame = aContent->GetPrimaryFrame();
         NS_ASSERTION(frame, "Should have frame here!");
       }
     }
 
     frame->CharacterDataChanged(aInfo);
 
     if (haveFirstLetterStyle) {
-      RecoverLetterFrames(block);
+      RecoverLetterFrames(block,
+                          ShouldHaveFirstLineStyle(block->GetContent(),
+                                                   block->StyleContext()));
     }
   }
 }
 
 void
 nsCSSFrameConstructor::BeginUpdate() {
   NS_ASSERTION(!nsContentUtils::IsSafeToRunScript(),
                "Someone forgot a script blocker");
@@ -12214,17 +12220,18 @@ nsCSSFrameConstructor::RemoveLetterFrame
     }
     continuation =
       static_cast<nsContainerFrame*>(continuation->GetNextContinuation());
   }  while (continuation);
 }
 
 // Fixup the letter frame situation for the given block
 void
-nsCSSFrameConstructor::RecoverLetterFrames(nsContainerFrame* aBlockFrame)
+nsCSSFrameConstructor::RecoverLetterFrames(nsContainerFrame* aBlockFrame,
+                                           bool aMayHaveFirstLine)
 {
   aBlockFrame =
     static_cast<nsContainerFrame*>(aBlockFrame->FirstContinuation());
   nsContainerFrame* continuation = aBlockFrame;
 
   nsContainerFrame* parentFrame = nullptr;
   nsIFrame* textFrame = nullptr;
   nsIFrame* prevFrame = nullptr;
@@ -12243,16 +12250,26 @@ nsCSSFrameConstructor::RecoverLetterFram
     continuation =
       static_cast<nsContainerFrame*>(continuation->GetNextContinuation());
   } while (continuation);
 
   if (parentFrame) {
     // Take the old textFrame out of the parents child list
     RemoveFrame(kPrincipalList, textFrame);
 
+    auto* restyleManager = RestyleManager();
+    if (aMayHaveFirstLine && restyleManager->IsServo()) {
+      // When we got the first-letter style from servo, it gave us the style not
+      // affected by the first-line bits.  So we need to reparent the new
+      // frames' styles to deal with that.
+      for (nsIFrame* f : letterFrames) {
+        restyleManager->ReparentStyleContext(f);
+      }
+    }
+
     // Insert in the letter frame(s)
     parentFrame->InsertFrames(kPrincipalList, prevFrame, letterFrames);
   }
 }
 
 //----------------------------------------------------------------------
 
 // listbox Widget Routines
--- a/layout/base/nsCSSFrameConstructor.h
+++ b/layout/base/nsCSSFrameConstructor.h
@@ -1993,17 +1993,18 @@ private:
                                     nsContainerFrame*  aParentFrame,
                                     nsIFrame*          aParentFrameList,
                                     nsContainerFrame** aModifiedParent,
                                     nsIFrame**         aTextFrame,
                                     nsIFrame**         aPrevFrame,
                                     nsFrameItems&      aLetterFrames,
                                     bool*              aStopLooking);
 
-  void RecoverLetterFrames(nsContainerFrame* aBlockFrame);
+  void RecoverLetterFrames(nsContainerFrame* aBlockFrame,
+                           bool aMayHaveFirstLine);
 
   //
   void RemoveLetterFrames(nsIPresShell*     aPresShell,
                           nsContainerFrame* aBlockFrame);
 
   // Recursive helper for RemoveLetterFrames
   void RemoveFirstLetterFrames(nsIPresShell*     aPresShell,
                                nsContainerFrame* aFrame,
new file mode 100644
--- /dev/null
+++ b/layout/reftests/first-letter/1385656-ref.html
@@ -0,0 +1,9 @@
+<!DOCTYPE html>
+<style>
+  div { border: 5px solid transparent; }
+  div::first-line { }
+  div::first-letter { border: inherit; border-color: yellow; }
+</style>
+<div>
+  Some text. Does the first letter still have a border?
+</div>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/first-letter/1385656.html
@@ -0,0 +1,16 @@
+<!DOCTYPE html>
+<style>
+  div { border: 5px solid transparent; }
+  div::first-line { }
+  div::first-letter { border: inherit; border-color: yellow; }
+</style>
+<div>
+  Some text.
+</div>
+<script>
+  onload = function() {
+    var div = document.querySelector("div");
+    window.width = div.offsetWidth; // Make sure we flush layout.
+    div.appendChild(document.createTextNode('Does the first letter still have a border?'));
+  }
+</script>
--- a/layout/reftests/first-letter/reftest.list
+++ b/layout/reftests/first-letter/reftest.list
@@ -68,8 +68,10 @@ fails-if((winWidget||cocoaWidget)&&!styl
 random-if(gtkWidget) random-if(winWidget&&!d2d) == font-text-styles-floater.html font-text-styles-floater-ref.html # bug 992846
 == inline-height-empty.html inline-height-empty-ref.html
 HTTP(..) == indic-clusters-1.html indic-clusters-1-ref.html
 == overflow-float-nooverflow.html overflow-float-nooverflow-ref.html
 == overflow-float-overflow.html overflow-float-overflow-notref.html
 == overflow-inline-nooverflow.html overflow-inline-nooverflow-ref.html
 != overflow-inline-overflow.html overflow-inline-overflow-notref.html
 == overflow-inline-overflow.html overflow-inline-overflow-ref.html
+
+== 1385656.html 1385656-ref.html