Skip to content

Commit

Permalink
Bug 1791226 - Don't paint line-clamped lines. r=layout-reviewers,dshin
Browse files Browse the repository at this point in the history 
This matches the proposal in
w3c/csswg-drafts#10816, and creates much better
behavior.

Differential Revision: https://phabricator.services.mozilla.com/D157578
  • Loading branch information
@emilio
emilio committed Oct 21, 2024
1 parent 2c8012c commit 8882e13
Show file tree
Hide file tree
Showing 6 changed files with 86 additions and 52 deletions.
4 changes: 2 additions & 2 deletions layout/generic/TextOverflow.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -810,15 +810,15 @@ bool TextOverflow::HasClippedTextOverflow(nsIFrame* aBlockFrame) {
/* static */
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,
bool aBeforeReflow) {
if (aBeforeReflow) {
return aBlockFrame->IsInLineClampContext();
}
return aBlockFrame->HasAnyStateBits(NS_BLOCK_HAS_LINE_CLAMP_ELLIPSIS);
return aBlockFrame->HasLineClampEllipsis();
}

/* static */
Expand Down
84 changes: 53 additions & 31 deletions layout/generic/nsBlockFrame.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1170,18 +1170,17 @@ static LogicalSize CalculateContainingBlockSizeForAbsolutes(
}

/**
* Returns aFrame if it is a non-BFC block frame, and null otherwise.
* Returns aFrame if it is an in-flow, non-BFC block frame, and null otherwise.
*
* This is used to determine whether to recurse into aFrame when applying
* -webkit-line-clamp.
*/
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 || block->HasAnyStateBits(NS_FRAME_OUT_OF_FLOW | NS_BLOCK_BFC)) {
return nullptr;
}
return nullptr;
return block;
}

static nsBlockFrame* GetAsLineClampDescendant(nsIFrame* aFrame) {
Expand Down Expand Up @@ -1343,28 +1342,30 @@ 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());
aFrame->SetHasLineClampEllipsis(false);
for (auto& line : aFrame->Lines()) {
if (line.HasLineClampEllipsis()) {
line.ClearHasLineClampEllipsis();
break;
}
}
return true;
}

if (aFrame->HasLineClampEllipsisDescendant()) {
aFrame->SetHasLineClampEllipsisDescendant(false);
for (nsIFrame* f : aFrame->PrincipalChildList()) {
if (nsBlockFrame* child = GetAsLineClampDescendant(f)) {
if (ClearLineClampEllipsis(child)) {
return true;
}
}
}
return false;
}

aFrame->RemoveStateBits(NS_BLOCK_HAS_LINE_CLAMP_ELLIPSIS);

for (auto& line : aFrame->Lines()) {
if (line.HasLineClampEllipsis()) {
line.ClearHasLineClampEllipsis();
return true;
}
}

// We didn't find a line with the ellipsis; it must have been deleted already.
return true;
return false;
}

void nsBlockFrame::ClearLineClampEllipsis() { ::ClearLineClampEllipsis(this); }
Expand Down Expand Up @@ -1869,6 +1870,12 @@ nsReflowStatus nsBlockFrame::TrialReflow(nsPresContext* aPresContext,
// overflow line lists being cleared out between reflow passes.
DrainOverflowLines();

// Clear any existing -webkit-line-clamp ellipsis if we're reflowing the
// line-clamp root.
if (IsLineClampRoot(this)) {
ClearLineClampEllipsis();
}

bool blockStartMarginRoot, blockEndMarginRoot;
IsMarginRoot(&blockStartMarginRoot, &blockEndMarginRoot);

Expand Down Expand Up @@ -2018,11 +2025,6 @@ nsReflowStatus nsBlockFrame::TrialReflow(nsPresContext* aPresContext,
// block-start padding.
}

// Clear any existing -webkit-line-clamp ellipsis.
if (aReflowInput.mStyleDisplay->mWebkitLineClamp) {
ClearLineClampEllipsis();
}

CheckFloats(state);

// Compute our final size (for this trial layout)
Expand Down Expand Up @@ -2052,8 +2054,8 @@ static nsLineBox* FindLineClampTarget(nsBlockFrame*& aFrame,
nsBlockFrame* aStopAtFrame,
StyleLineClamp aLineNumber) {
MOZ_ASSERT(aLineNumber > 0);
MOZ_ASSERT(!aFrame->HasAnyStateBits(NS_BLOCK_HAS_LINE_CLAMP_ELLIPSIS),
"Should have been removed earlier in nsBlockReflow::Reflow");
MOZ_ASSERT(!aFrame->HasLineClampEllipsis(),
"Should have been removed earlier");

nsLineBox* target = nullptr;
nsBlockFrame* targetFrame = nullptr;
Expand Down Expand Up @@ -2114,11 +2116,16 @@ nscoord nsBlockFrame::ApplyLineClamp(nscoord aContentBlockEndEdge) {

// Mark the line as having an ellipsis so that TextOverflow will render it.
line->SetHasLineClampEllipsis();
target->AddStateBits(NS_BLOCK_HAS_LINE_CLAMP_ELLIPSIS);
target->SetHasLineClampEllipsis(true);

// Translate the b-end edge of the line up to aFrame's space.
nscoord edge = line->BEnd();
for (nsIFrame* f = target; f; f = f->GetParent()) {
MOZ_ASSERT(f->IsBlockFrameOrSubclass(),
"GetAsLineClampDescendant guarantees this");
if (f != target) {
static_cast<nsBlockFrame*>(f)->SetHasLineClampEllipsisDescendant(true);
}
if (f == this) {
break;
}
Expand Down Expand Up @@ -7637,7 +7644,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++;
Expand Down Expand Up @@ -7670,6 +7677,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);
Expand Down Expand Up @@ -7774,12 +7789,14 @@ void nsBlockFrame::BuildDisplayList(nsDisplayListBuilder* aBuilder,
// runs of text as a whole, which requires we iterate through all lines
// to find our backplate size.
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) {
Expand All @@ -7794,7 +7811,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);
}
}
}
Expand Down Expand Up @@ -7825,7 +7843,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()) {
Expand Down Expand Up @@ -7856,6 +7874,10 @@ void nsBlockFrame::BuildDisplayList(nsDisplayListBuilder* aBuilder,
}
}
}
foundClamp = foundClamp || line->HasLineClampEllipsis();
if (foundClamp) {
break;
}
lineCount++;
}

Expand Down
22 changes: 22 additions & 0 deletions layout/generic/nsBlockFrame.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -681,6 +681,28 @@ class nsBlockFrame : public nsContainerFrame {
* This function is O(1).
*/
bool MaybeHasFloats() const;
/**
* This indicates that exactly one line in this block has the
* LineClampEllipsis flag set, and that such a line must be found
* and have that flag cleared when reflowing this element's nearest legacy box
* container.
*/
bool HasLineClampEllipsis() const {
return HasAnyStateBits(NS_BLOCK_HAS_LINE_CLAMP_ELLIPSIS);
}
/**
* This indicates that we have a descendant in our block formatting context
* that has such a line.
*/
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;
Expand Down
16 changes: 6 additions & 10 deletions layout/generic/nsFrameStateBits.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -562,18 +562,14 @@ FRAME_STATE_BIT(Block, 28, NS_BLOCK_HAS_MARKER)
// continuation chain or none of them.
FRAME_STATE_BIT(Block, 29, NS_BLOCK_NEEDS_BIDI_RESOLUTION)

// bits 30 and 31 free.

// NS_BLOCK_HAS_LINE_CLAMP_ELLIPSIS indicates that exactly one line in this
// block has the LineClampEllipsis flag set, and that such a line must be found
// and have that flag cleared when reflowing this element's nearest legacy box
// container.
FRAME_STATE_BIT(Block, 60, NS_BLOCK_HAS_LINE_CLAMP_ELLIPSIS)
// See nsBlockFrame.h for docs
FRAME_STATE_BIT(Block, 30, NS_BLOCK_HAS_LINE_CLAMP_ELLIPSIS)
FRAME_STATE_BIT(Block, 31, NS_BLOCK_HAS_LINE_CLAMP_ELLIPSIS_DESCENDANT)

// This block has had a child marked dirty, so before we reflow we need
// to look through the lines to find any such children and mark
// appropriate lines dirty.
FRAME_STATE_BIT(Block, 61, NS_BLOCK_LOOK_FOR_DIRTY_FRAMES)
FRAME_STATE_BIT(Block, 60, NS_BLOCK_LOOK_FOR_DIRTY_FRAMES)

// Are our cached intrinsic inline sizes for font size inflation? i.e., what was
// the current state of GetPresContext()->mInflationDisabledForShrinkWrap at the
Expand All @@ -583,13 +579,13 @@ FRAME_STATE_BIT(Block, 61, NS_BLOCK_LOOK_FOR_DIRTY_FRAMES)
// to track this because it's the only thing that caches intrinsic inline sizes
// that lives inside of things (form controls) that do intrinsic sizing with
// font inflation enabled.
FRAME_STATE_BIT(Block, 62, NS_BLOCK_INTRINSICS_INFLATED)
FRAME_STATE_BIT(Block, 61, NS_BLOCK_INTRINSICS_INFLATED)

// NS_BLOCK_HAS_FIRST_LETTER_CHILD means that there is an inflow first-letter
// frame among the block's descendants. If there is a floating first-letter
// frame, or the block has first-letter style but has no first letter, this
// bit is not set. This bit is set on the first continuation only.
FRAME_STATE_BIT(Block, 63, NS_BLOCK_HAS_FIRST_LETTER_CHILD)
FRAME_STATE_BIT(Block, 62, NS_BLOCK_HAS_FIRST_LETTER_CHILD)

// == Frame state bits that apply to image frames =============================

Expand Down
4 changes: 1 addition & 3 deletions ...g/web-platform/tests/css/css-overflow/line-clamp/reference/webkit-line-clamp-040-ref.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,4 @@
</style>
<div class="clamp">Line 1
Line 2…
Line 3
Line 4
Line 5</div>
</div>
8 changes: 2 additions & 6 deletions testing/web-platform/tests/css/css-overflow/line-clamp/webkit-line-clamp-050.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,11 @@
border: medium solid green;
padding: 15px;
}
span {
/* TODO: Remove once we don't paint clamped lines */
color: transparent;
}
</style>
<div class="clamp">
Line1
<div>Line2<br><span>Line3</span></div>
<span>Line4</span>
<div>Line2<br>Line3</div>
Line4
<div>Line5<br>Line6</div>
Line7
</div>

0 comments on commit 8882e13

Please sign in to comment.