From 422c4815a9008a6a778021615f367ea3fb8e62a9 Mon Sep 17 00:00:00 2001 From: Marco Castelluccio Date: Tue, 22 Oct 2024 22:00:23 +0000 Subject: [PATCH] Bug 1791226 - Don't paint line-clamped lines. r=layout-reviewers,dshin This matches the proposal in https://github.com/w3c/csswg-drafts/pull/10816, and creates much better behavior. Differential Revision: https://phabricator.services.mozilla.com/D157578 UltraBlame original commit: c6ce92301ca11292a5dd25f07a80243592c94e5a --- layout/generic/TextOverflow.cpp | 4 +- layout/generic/nsBlockFrame.cpp | 77 ++++++++++++------- layout/generic/nsBlockFrame.h | 22 ++++++ layout/generic/nsFrameStateBits.h | 14 ++-- .../reference/webkit-line-clamp-040-ref.html | 4 +- .../line-clamp/webkit-line-clamp-050.html | 8 +- 6 files changed, 83 insertions(+), 46 deletions(-) diff --git a/layout/generic/TextOverflow.cpp b/layout/generic/TextOverflow.cpp index eeab0856d927..4a8725ccb24b 100644 --- a/layout/generic/TextOverflow.cpp +++ b/layout/generic/TextOverflow.cpp @@ -810,7 +810,7 @@ bool TextOverflow::HasClippedTextOverflow(nsIFrame* aBlockFrame) { bool TextOverflow::HasBlockEllipsis(nsIFrame* aBlockFrame) { nsBlockFrame* f = do_QueryFrame(aBlockFrame); - return f && f->HasAnyStateBits(NS_BLOCK_HAS_LINE_CLAMP_ELLIPSIS); + return f && f->HasLineClampEllipsis(); } static bool BlockCanHaveLineClampEllipsis(nsBlockFrame* aBlockFrame, @@ -818,7 +818,7 @@ static bool BlockCanHaveLineClampEllipsis(nsBlockFrame* aBlockFrame, if (aBeforeReflow) { return aBlockFrame->IsInLineClampContext(); } - return aBlockFrame->HasAnyStateBits(NS_BLOCK_HAS_LINE_CLAMP_ELLIPSIS); + return aBlockFrame->HasLineClampEllipsis(); } diff --git a/layout/generic/nsBlockFrame.cpp b/layout/generic/nsBlockFrame.cpp index 261726750654..a2cbbe71e11d 100644 --- a/layout/generic/nsBlockFrame.cpp +++ b/layout/generic/nsBlockFrame.cpp @@ -1176,12 +1176,17 @@ static LogicalSize CalculateContainingBlockSizeForAbsolutes( static const nsBlockFrame* GetAsLineClampDescendant(const nsIFrame* aFrame) { - if (const nsBlockFrame* block = do_QueryFrame(aFrame)) { - if (!block->HasAnyStateBits(NS_BLOCK_BFC)) { - return block; - } + const nsBlockFrame* block = do_QueryFrame(aFrame); + if (!block) { + return nullptr; } - return nullptr; + if (block->HasAllStateBits(NS_FRAME_OUT_OF_FLOW)) { + return nullptr; + } + if (block->HasAllStateBits(NS_BLOCK_BFC)) { + return nullptr; + } + return block; } static nsBlockFrame* GetAsLineClampDescendant(nsIFrame* aFrame) { @@ -1343,7 +1348,19 @@ class MOZ_RAII LineClampLineIterator { }; static bool ClearLineClampEllipsis(nsBlockFrame* aFrame) { - if (!aFrame->HasAnyStateBits(NS_BLOCK_HAS_LINE_CLAMP_ELLIPSIS)) { + if (aFrame->HasLineClampEllipsis()) { + MOZ_ASSERT(!aFrame->HasLineClampEllipsisDescendant()); + for (auto& line : aFrame->Lines()) { + if (line.HasLineClampEllipsis()) { + line.ClearHasLineClampEllipsis(); + aFrame->SetHasLineClampEllipsis(false); + return true; + } + } + } + + if (aFrame->HasLineClampEllipsisDescendant()) { + aFrame->SetHasLineClampEllipsisDescendant(false); for (nsIFrame* f : aFrame->PrincipalChildList()) { if (nsBlockFrame* child = GetAsLineClampDescendant(f)) { if (ClearLineClampEllipsis(child)) { @@ -1351,20 +1368,10 @@ static bool ClearLineClampEllipsis(nsBlockFrame* aFrame) { } } } - return false; - } - - aFrame->RemoveStateBits(NS_BLOCK_HAS_LINE_CLAMP_ELLIPSIS); - - for (auto& line : aFrame->Lines()) { - if (line.HasLineClampEllipsis()) { - line.ClearHasLineClampEllipsis(); - return true; - } } - return true; + return false; } void nsBlockFrame::ClearLineClampEllipsis() { ::ClearLineClampEllipsis(this); } @@ -2019,9 +2026,7 @@ nsReflowStatus nsBlockFrame::TrialReflow(nsPresContext* aPresContext, } - if (aReflowInput.mStyleDisplay->mWebkitLineClamp) { - ClearLineClampEllipsis(); - } + ClearLineClampEllipsis(); CheckFloats(state); @@ -2052,7 +2057,7 @@ static nsLineBox* FindLineClampTarget(nsBlockFrame*& aFrame, nsBlockFrame* aStopAtFrame, StyleLineClamp aLineNumber) { MOZ_ASSERT(aLineNumber > 0); - MOZ_ASSERT(!aFrame->HasAnyStateBits(NS_BLOCK_HAS_LINE_CLAMP_ELLIPSIS), + MOZ_ASSERT(!aFrame->HasLineClampEllipsis(), "Should have been removed earlier in nsBlockReflow::Reflow"); nsLineBox* target = nullptr; @@ -2114,11 +2119,16 @@ nscoord nsBlockFrame::ApplyLineClamp(nscoord aContentBlockEndEdge) { line->SetHasLineClampEllipsis(); - target->AddStateBits(NS_BLOCK_HAS_LINE_CLAMP_ELLIPSIS); + target->SetHasLineClampEllipsis(true); nscoord edge = line->BEnd(); for (nsIFrame* f = target; f; f = f->GetParent()) { + MOZ_ASSERT(f->IsBlockFrameOrSubclass(), + "GetAsLineClampDescendant guarantees this"); + if (f != target) { + static_cast(f)->SetHasLineClampEllipsisDescendant(true); + } if (f == this) { break; } @@ -7637,7 +7647,7 @@ static void DisplayLine(nsDisplayListBuilder* aBuilder, const bool aLineInLine, const nsDisplayListSet& aLists, nsBlockFrame* aFrame, TextOverflow* aTextOverflow, uint32_t aLineNumberForTextOverflow, int32_t aDepth, - int32_t& aDrawnLines) { + int32_t& aDrawnLines, bool& aFoundLineClamp) { #ifdef DEBUG if (nsBlockFrame::gLamePaintMetrics) { aDrawnLines++; @@ -7670,6 +7680,14 @@ static void DisplayLine(nsDisplayListBuilder* aBuilder, kid = kid->GetNextSibling(); } + if (aFrame->HasLineClampEllipsisDescendant() && !aLineInLine) { + if (nsBlockFrame* f = GetAsLineClampDescendant(aLine->mFirstChild)) { + if (f->HasLineClampEllipsis() || f->HasLineClampEllipsisDescendant()) { + aFoundLineClamp = true; + } + } + } + if (aTextOverflow && aLineInLine) { aTextOverflow->ProcessLine(collection, aLine.get(), aLineNumberForTextOverflow); @@ -7774,12 +7792,14 @@ void nsBlockFrame::BuildDisplayList(nsDisplayListBuilder* aBuilder, nsLineBox* cursor = - (hasDescendantPlaceHolders || textOverflow.isSome() || backplateColor) + (hasDescendantPlaceHolders || textOverflow.isSome() || backplateColor || + HasLineClampEllipsis() || HasLineClampEllipsisDescendant()) ? nullptr : GetFirstLineContaining(aBuilder->GetDirtyRect().y); LineIterator line_end = LinesEnd(); TextOverflow* textOverflowPtr = textOverflow.ptrOr(nullptr); + bool foundClamp = false; if (cursor) { for (LineIterator line = mLines.begin(cursor); line != line_end; ++line) { @@ -7794,7 +7814,8 @@ void nsBlockFrame::BuildDisplayList(nsDisplayListBuilder* aBuilder, if (ShouldDescendIntoLine(lineArea)) { DisplayLine(aBuilder, line, line->IsInline(), aLists, this, nullptr, - 0, depth, drawnLines); + 0, depth, drawnLines, foundClamp); + MOZ_ASSERT(!foundClamp); } } } @@ -7825,7 +7846,7 @@ void nsBlockFrame::BuildDisplayList(nsDisplayListBuilder* aBuilder, if ((lineInLine && textOverflowPtr) || ShouldDescendIntoLine(lineArea)) { DisplayLine(aBuilder, line, lineInLine, aLists, this, textOverflowPtr, - lineCount, depth, drawnLines); + lineCount, depth, drawnLines, foundClamp); } if (!lineInLine && !curBackplateArea.IsEmpty()) { @@ -7856,6 +7877,10 @@ void nsBlockFrame::BuildDisplayList(nsDisplayListBuilder* aBuilder, } } } + foundClamp = foundClamp || line->HasLineClampEllipsis(); + if (foundClamp) { + break; + } lineCount++; } diff --git a/layout/generic/nsBlockFrame.h b/layout/generic/nsBlockFrame.h index 4658638f1fad..89bc66166357 100644 --- a/layout/generic/nsBlockFrame.h +++ b/layout/generic/nsBlockFrame.h @@ -681,6 +681,28 @@ class nsBlockFrame : public nsContainerFrame { bool MaybeHasFloats() const; + + + + + + + bool HasLineClampEllipsis() const { + return HasAnyStateBits(NS_BLOCK_HAS_LINE_CLAMP_ELLIPSIS); + } + + + + + bool HasLineClampEllipsisDescendant() const { + return HasAnyStateBits(NS_BLOCK_HAS_LINE_CLAMP_ELLIPSIS_DESCENDANT); + } + void SetHasLineClampEllipsis(bool aValue) { + AddOrRemoveStateBits(NS_BLOCK_HAS_LINE_CLAMP_ELLIPSIS, aValue); + } + void SetHasLineClampEllipsisDescendant(bool aValue) { + AddOrRemoveStateBits(NS_BLOCK_HAS_LINE_CLAMP_ELLIPSIS_DESCENDANT, aValue); + } protected: nsBlockFrame* GetLineClampRoot() const; diff --git a/layout/generic/nsFrameStateBits.h b/layout/generic/nsFrameStateBits.h index d9d5a6183908..16f668a97105 100644 --- a/layout/generic/nsFrameStateBits.h +++ b/layout/generic/nsFrameStateBits.h @@ -563,33 +563,29 @@ FRAME_STATE_BIT(Block, 28, NS_BLOCK_HAS_MARKER) FRAME_STATE_BIT(Block, 29, NS_BLOCK_NEEDS_BIDI_RESOLUTION) +FRAME_STATE_BIT(Block, 30, NS_BLOCK_HAS_LINE_CLAMP_ELLIPSIS) +FRAME_STATE_BIT(Block, 31, NS_BLOCK_HAS_LINE_CLAMP_ELLIPSIS_DESCENDANT) +FRAME_STATE_BIT(Block, 60, NS_BLOCK_LOOK_FOR_DIRTY_FRAMES) -FRAME_STATE_BIT(Block, 60, NS_BLOCK_HAS_LINE_CLAMP_ELLIPSIS) -FRAME_STATE_BIT(Block, 61, NS_BLOCK_LOOK_FOR_DIRTY_FRAMES) +FRAME_STATE_BIT(Block, 61, NS_BLOCK_INTRINSICS_INFLATED) -FRAME_STATE_BIT(Block, 62, NS_BLOCK_INTRINSICS_INFLATED) - - - - - -FRAME_STATE_BIT(Block, 63, NS_BLOCK_HAS_FIRST_LETTER_CHILD) +FRAME_STATE_BIT(Block, 62, NS_BLOCK_HAS_FIRST_LETTER_CHILD) diff --git a/testing/web-platform/tests/css/css-overflow/line-clamp/reference/webkit-line-clamp-040-ref.html b/testing/web-platform/tests/css/css-overflow/line-clamp/reference/webkit-line-clamp-040-ref.html index f55be86e5461..0e94ba746120 100644 --- a/testing/web-platform/tests/css/css-overflow/line-clamp/reference/webkit-line-clamp-040-ref.html +++ b/testing/web-platform/tests/css/css-overflow/line-clamp/reference/webkit-line-clamp-040-ref.html @@ -11,6 +11,4 @@
Line 1 Line 2… -Line 3 -Line 4 -Line 5
+ diff --git a/testing/web-platform/tests/css/css-overflow/line-clamp/webkit-line-clamp-050.html b/testing/web-platform/tests/css/css-overflow/line-clamp/webkit-line-clamp-050.html index 973871b72d85..8757834349fc 100644 --- a/testing/web-platform/tests/css/css-overflow/line-clamp/webkit-line-clamp-050.html +++ b/testing/web-platform/tests/css/css-overflow/line-clamp/webkit-line-clamp-050.html @@ -16,15 +16,11 @@ border: medium solid green; padding: 15px; } - span { - /* TODO: Remove once we don't paint clamped lines */ - color: transparent; - }
Line1 -
Line2
Line3
- Line4 +
Line2
Line3
+ Line4
Line5
Line6
Line7