From 5bd9b06ca045201b8b4c90732fc56e07361073da Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Wed, 28 Jun 2017 22:19:35 -0700 Subject: [PATCH] Bug 1376640. Fix dynamic updates when an inline that sits between a first-letter and its block changes style. r=heycam MozReview-Commit-ID: 8osMUpYVRvR --- layout/base/ServoRestyleManager.cpp | 30 ++++++++++++++----- layout/base/nsCSSFrameConstructor.cpp | 6 ++++ layout/generic/nsBlockFrame.cpp | 18 +++++------ layout/generic/nsBlockFrame.h | 12 ++++---- layout/generic/nsContainerFrame.h | 10 +++++++ layout/generic/nsFrame.cpp | 11 +++++-- layout/generic/nsIFrame.h | 22 ++++++++++++-- .../reftests/first-letter/dynamic-4-ref.html | 10 +++++++ layout/reftests/first-letter/dynamic-4.html | 23 ++++++++++++++ layout/reftests/first-letter/reftest.list | 1 + 10 files changed, 115 insertions(+), 28 deletions(-) create mode 100644 layout/reftests/first-letter/dynamic-4-ref.html create mode 100644 layout/reftests/first-letter/dynamic-4.html diff --git a/layout/base/ServoRestyleManager.cpp b/layout/base/ServoRestyleManager.cpp index c6335aed1d7c..bb43edabe64f 100644 --- a/layout/base/ServoRestyleManager.cpp +++ b/layout/base/ServoRestyleManager.cpp @@ -15,7 +15,6 @@ #include "mozilla/dom/ElementInlines.h" #include "nsBlockFrame.h" #include "nsBulletFrame.h" -#include "nsFirstLetterFrame.h" #include "nsPlaceholderFrame.h" #include "nsContentUtils.h" #include "nsCSSFrameConstructor.h" @@ -56,13 +55,6 @@ ExpectedOwnerForChild(const nsIFrame& aFrame) } const nsIFrame* parent = FirstContinuationOrPartOfIBSplit(aFrame.GetParent()); - if (nsFirstLetterFrame* fl = do_QueryFrame(const_cast(&aFrame))) { - Unused << fl; - while (!parent->IsFrameOfType(nsIFrame::eBlockFrame)) { - parent = FirstContinuationOrPartOfIBSplit(parent->GetParent()); - } - return parent; - } if (aFrame.IsTableFrame()) { MOZ_ASSERT(parent->IsTableWrapperFrame()); @@ -398,10 +390,32 @@ UpdateBackdropIfNeeded(nsIFrame* aFrame, aFrame->UpdateStyleOfOwnedChildFrame(backdropFrame, newContext, state); } +static void +UpdateFirstLetterIfNeeded(nsIFrame* aFrame, ServoRestyleState& aRestyleState) +{ + if (!aFrame->HasFirstLetterChild()) { + return; + } + + // We need to find the block the first-letter is associated with so we can + // find the right element for the first-letter's style resolution. Might as + // well just delegate the whole thing to that block. + nsIFrame* block = aFrame; + while (!block->IsFrameOfType(nsIFrame::eBlockFrame)) { + block = block->GetParent(); + } + static_cast(block->FirstContinuation())-> + UpdateFirstLetterStyle(aRestyleState); +} + static void UpdateFramePseudoElementStyles(nsIFrame* aFrame, ServoRestyleState& aRestyleState) { + // first-letter needs to be updated before first-line, because first-line can + // change the style of the first-letter. + UpdateFirstLetterIfNeeded(aFrame, aRestyleState); + if (aFrame->IsFrameOfType(nsIFrame::eBlockFrame)) { static_cast(aFrame)->UpdatePseudoElementStyles(aRestyleState); } diff --git a/layout/base/nsCSSFrameConstructor.cpp b/layout/base/nsCSSFrameConstructor.cpp index 1f2ae2ff6452..111672f3c0da 100644 --- a/layout/base/nsCSSFrameConstructor.cpp +++ b/layout/base/nsCSSFrameConstructor.cpp @@ -11811,6 +11811,8 @@ nsCSSFrameConstructor::CreateLetterFrame(nsContainerFrame* aBlockFrame, } MOZ_ASSERT(!aBlockFrame->GetPrevContinuation(), "Setting up a first-letter frame on a non-first block continuation?"); + auto parent = static_cast(aParentFrame->FirstContinuation()); + parent->SetHasFirstLetterChild(); aBlockFrame->SetProperty(nsContainerFrame::FirstLetterProperty(), letterFrame); aTextContent->SetPrimaryFrame(textFrame); @@ -11956,6 +11958,8 @@ nsCSSFrameConstructor::RemoveFloatingFirstLetterFrames( // Somethings really wrong return; } + static_cast(parentFrame->FirstContinuation())-> + ClearHasFirstLetterChild(); // Create a new text frame with the right style context that maps // all of the content that was previously part of the letter frame @@ -12021,6 +12025,8 @@ nsCSSFrameConstructor::RemoveFirstLetterFrames(nsIPresShell* aPresShell, while (kid) { if (kid->IsLetterFrame()) { // Bingo. Found it. First steal away the text frame. + static_cast(aFrame->FirstContinuation())-> + ClearHasFirstLetterChild(); nsIFrame* textFrame = kid->PrincipalChildList().FirstChild(); if (!textFrame) { break; diff --git a/layout/generic/nsBlockFrame.cpp b/layout/generic/nsBlockFrame.cpp index 9a3f6dd55fd2..676f34b8f6f3 100644 --- a/layout/generic/nsBlockFrame.cpp +++ b/layout/generic/nsBlockFrame.cpp @@ -5626,12 +5626,16 @@ nsBlockInFlowLineIterator::nsBlockInFlowLineIterator(nsBlockFrame* aFrame, } void -nsBlockFrame::UpdateFirstLetterStyle(nsIFrame* aLetterFrame, - ServoRestyleState& aRestyleState) +nsBlockFrame::UpdateFirstLetterStyle(ServoRestyleState& aRestyleState) { + nsIFrame* letterFrame = GetFirstLetter(); + if (!letterFrame) { + return; + } + // Figure out what the right style parent is. This needs to match // nsCSSFrameConstructor::CreateLetterFrame. - nsIFrame* inFlowFrame = aLetterFrame; + nsIFrame* inFlowFrame = letterFrame; if (inFlowFrame->GetStateBits() & NS_FRAME_OUT_OF_FLOW) { inFlowFrame = inFlowFrame->GetPlaceholderFrame(); } @@ -5649,13 +5653,13 @@ nsBlockFrame::UpdateFirstLetterStyle(nsIFrame* aLetterFrame, // styles: those will be handled by the styleParent already. RefPtr continuationStyle = aRestyleState.StyleSet().ResolveStyleForFirstLetterContinuation(parentStyle); - UpdateStyleOfOwnedChildFrame(aLetterFrame, firstLetterStyle, aRestyleState, + UpdateStyleOfOwnedChildFrame(letterFrame, firstLetterStyle, aRestyleState, Some(continuationStyle.get())); // We also want to update the style on the textframe inside the first-letter. // We don't need to compute a changehint for this, though, since any changes // to it are handled by the first-letter anyway. - nsIFrame* textFrame = aLetterFrame->PrincipalChildList().FirstChild(); + nsIFrame* textFrame = letterFrame->PrincipalChildList().FirstChild(); RefPtr firstTextStyle = aRestyleState.StyleSet().ResolveStyleForText(textFrame->GetContent(), firstLetterStyle); @@ -7564,10 +7568,6 @@ nsBlockFrame::UpdatePseudoElementStyles(ServoRestyleState& aRestyleState) ResolveBulletStyle(type, &aRestyleState.StyleSet()); UpdateStyleOfOwnedChildFrame(bullet, newBulletStyle, aRestyleState); } - - if (nsIFrame* firstLetter = GetFirstLetter()) { - UpdateFirstLetterStyle(firstLetter, aRestyleState); - } } already_AddRefed diff --git a/layout/generic/nsBlockFrame.h b/layout/generic/nsBlockFrame.h index 08a9beecbd5c..050283fdc80e 100644 --- a/layout/generic/nsBlockFrame.h +++ b/layout/generic/nsBlockFrame.h @@ -403,11 +403,15 @@ public: }; /** - * Update the styles of our various pseudo-elements (bullets, first-letter, - * first-line, etc). + * Update the styles of our various pseudo-elements (bullets, first-line, + * etc, but _not_ first-letter). */ void UpdatePseudoElementStyles(mozilla::ServoRestyleState& aRestyleState); + // Update our first-letter styles during stylo post-traversal. This needs to + // be done at a slightly different time than our other pseudo-elements. + void UpdateFirstLetterStyle(mozilla::ServoRestyleState& aRestyleState); + protected: explicit nsBlockFrame(nsStyleContext* aContext, ClassID aID = kClassID) : nsContainerFrame(aContext, aID) @@ -923,10 +927,6 @@ protected: mozilla::CSSPseudoElementType aType, mozilla::StyleSetHandle aStyleSet); - // Update our first-letter styles during stylo post-traversal. - void UpdateFirstLetterStyle(nsIFrame* aLetterFrame, - mozilla::ServoRestyleState& aRestyleState); - #ifdef DEBUG void VerifyLines(bool aFinalCheckOK); void VerifyOverflowSituation(); diff --git a/layout/generic/nsContainerFrame.h b/layout/generic/nsContainerFrame.h index 06605bdeda75..432b1361e2a1 100644 --- a/layout/generic/nsContainerFrame.h +++ b/layout/generic/nsContainerFrame.h @@ -527,6 +527,16 @@ public: // have arbitrary nsContainerFrames. NS_DECLARE_FRAME_PROPERTY_WITHOUT_DTOR(FirstLetterProperty, nsIFrame) + void SetHasFirstLetterChild() + { + mHasFirstLetterChild = true; + } + + void ClearHasFirstLetterChild() + { + mHasFirstLetterChild = false; + } + #ifdef DEBUG // Use this to suppress the CRAZY_SIZE assertions. NS_DECLARE_FRAME_PROPERTY_SMALL_VALUE(DebugReflowingWithInfiniteISize, bool) diff --git a/layout/generic/nsFrame.cpp b/layout/generic/nsFrame.cpp index 64a6ce1e6851..6a8f60ee1151 100644 --- a/layout/generic/nsFrame.cpp +++ b/layout/generic/nsFrame.cpp @@ -10248,7 +10248,7 @@ nsIFrame::UpdateStyleOfChildAnonBox(nsIFrame* aChildFrame, } } -nsChangeHint +/* static */ nsChangeHint nsIFrame::UpdateStyleOfOwnedChildFrame( nsIFrame* aChildFrame, nsStyleContext* aNewStyleContext, @@ -10270,8 +10270,13 @@ nsIFrame::UpdateStyleOfOwnedChildFrame( aNewStyleContext, &equalStructs, &samePointerStructs); - childHint = NS_RemoveSubsumedHints( - childHint, aRestyleState.ChangesHandledFor(*aChildFrame)); + // If aChildFrame is out of flow, then aRestyleState's "changes handled by the + // parent" doesn't apply to it, because it may have some other parent in the + // frame tree. + if (!aChildFrame->HasAnyStateBits(NS_FRAME_OUT_OF_FLOW)) { + childHint = NS_RemoveSubsumedHints( + childHint, aRestyleState.ChangesHandledFor(*aChildFrame)); + } if (childHint) { if (childHint & nsChangeHint_ReconstructFrame) { // If we generate a reconstruct here, remove any non-reconstruct hints we diff --git a/layout/generic/nsIFrame.h b/layout/generic/nsIFrame.h index 416fabf77508..41ce525f05b6 100644 --- a/layout/generic/nsIFrame.h +++ b/layout/generic/nsIFrame.h @@ -618,6 +618,7 @@ public: , mClass(aID) , mMayHaveRoundedCorners(false) , mHasImageRequest(false) + , mHasFirstLetterChild(false) { mozilla::PodZero(&mOverflow); } @@ -3354,7 +3355,7 @@ public: // only computed based on aNewStyleContext. // // Returns the generated change hint for the frame. - nsChangeHint UpdateStyleOfOwnedChildFrame( + static nsChangeHint UpdateStyleOfOwnedChildFrame( nsIFrame* aChildFrame, nsStyleContext* aNewStyleContext, mozilla::ServoRestyleState& aRestyleState, @@ -3979,6 +3980,15 @@ public: */ void SetHasImageRequest(bool aHasRequest) { mHasImageRequest = aHasRequest; } + /** + * Whether this frame has a first-letter child. If it does, the frame is + * actually an nsContainerFrame and the first-letter frame can be gotten by + * walking up to the nearest ancestor blockframe and getting its first + * continuation's nsContainerFrame::FirstLetterProperty() property. This will + * only return true for the first continuation of the first-letter's parent. + */ + bool HasFirstLetterChild() const { return mHasFirstLetterChild; } + /** * If this returns true, the frame it's called on should get the * NS_FRAME_HAS_DIRTY_CHILDREN bit set on it by the caller; either directly @@ -4117,7 +4127,15 @@ protected: */ bool mHasImageRequest : 1; - // There is a 14-bit gap left here. + /** + * True if this frame has a continuation that has a first-letter frame, or its + * placeholder, as a child. In that case this frame has a blockframe ancestor + * that has the first-letter frame hanging off it in the + * nsContainerFrame::FirstLetterProperty() property. + */ + bool mHasFirstLetterChild : 1; + + // There is a 13-bit gap left here. // Helpers /** diff --git a/layout/reftests/first-letter/dynamic-4-ref.html b/layout/reftests/first-letter/dynamic-4-ref.html new file mode 100644 index 000000000000..6ae1622cab28 --- /dev/null +++ b/layout/reftests/first-letter/dynamic-4-ref.html @@ -0,0 +1,10 @@ + + + +
+ X +
+ diff --git a/layout/reftests/first-letter/dynamic-4.html b/layout/reftests/first-letter/dynamic-4.html new file mode 100644 index 000000000000..636859e95278 --- /dev/null +++ b/layout/reftests/first-letter/dynamic-4.html @@ -0,0 +1,23 @@ + + + +
+ X +
+ + diff --git a/layout/reftests/first-letter/reftest.list b/layout/reftests/first-letter/reftest.list index 1ac87c16461b..6e188af1e86e 100644 --- a/layout/reftests/first-letter/reftest.list +++ b/layout/reftests/first-letter/reftest.list @@ -26,6 +26,7 @@ fails-if(!stylo) fails-if(styloVsGecko) == dynamic-1.html dynamic-1-ref.html # b random-if(d2d) == dynamic-2.html dynamic-2-ref.html == dynamic-3a.html dynamic-3-ref.html == dynamic-3b.html dynamic-3-ref.html +== dynamic-4.html dynamic-4-ref.html == 23605-1.html 23605-1-ref.html == 23605-2.html 23605-2-ref.html == 23605-3.html 23605-3-ref.html