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

Author: emilio

layout/generic/TextOverflow.cpp

@@ -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 */

layout/generic/nsBlockFrame.cpp

@@ -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) {
@@ -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); }
@@ -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);
 
@@ -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)
@@ -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;
@@ -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(f)->SetHasLineClampEllipsisDescendant(true);
+    }
     if (f == this) {
       break;
     }
@@ -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++;
@@ -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);
@@ -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) {
@@ -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);
         }
       }
     }
@@ -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()) {
@@ -7856,6 +7874,10 @@ void nsBlockFrame::BuildDisplayList(nsDisplayListBuilder* aBuilder,
           }
         }
       }
+      foundClamp = foundClamp || line->HasLineClampEllipsis();
+      if (foundClamp) {
+        break;
+      }
       lineCount++;
     }
 

layout/generic/nsBlockFrame.h

@@ -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;

layout/generic/nsFrameStateBits.h

@@ -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
@@ -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 =============================
 

testing/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 5div>
+div>

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>Line3span>div>
-  <span>Line4span>
+  <div>Line2<br>Line3div>
+  Line4
   <div>Line5<br>Line6div>
   Line7
 div>