Bug 1791226 - Don't paint line-clamped lines. r=layout-reviewers,dshin

This matches the proposal in w3c/csswg-drafts#10816, and creates much better behavior. Differential Revision: https://phabricator.services.mozilla.com/D157578 UltraBlame original commit: c6ce92301ca11292a5dd25f07a80243592c94e5a
marco-c · Oct 22, 2024 · 422c481 · 422c481
1 parent 84d035b
commit 422c481
Show file tree

Hide file tree

Showing 6 changed files with 83 additions and 46 deletions.
diff --git a/layout/generic/TextOverflow.cpp b/layout/generic/TextOverflow.cpp
@@ -810,15 +810,15 @@ 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,
                                           bool aBeforeReflow) {
   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
@@ -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,28 +1348,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());
+    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)) {
           return true;
         }
       }
     }
-    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<nsBlockFrame*>(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
@@ -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
@@ -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/...g/web-platform/tests/css/css-overflow/line-clamp/reference/webkit-line-clamp-040-ref.html b/...g/web-platform/tests/css/css-overflow/line-clamp/reference/webkit-line-clamp-040-ref.html
@@ -11,6 +11,4 @@
 </style>
 <div class="clamp">Line 1
 Line 2…
-Line 3
-Line 4
-Line 5</div>
+</div>
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
@@ -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>