[webkit-changes] [WebKit/WebKit] 521f00: [IFC][line-clamp](REGRESSION 264048 at main) Yahoo.co...

Alan Baradlay noreply at github.com
Fri Jun 2 05:47:45 PDT 2023


  Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: 521f0038f2b366e75060b9f76f7997c6f4b2a383
      https://github.com/WebKit/WebKit/commit/521f0038f2b366e75060b9f76f7997c6f4b2a383
  Author: Alan Baradlay <zalan at apple.com>
  Date:   2023-06-02 (Fri, 02 Jun 2023)

  Changed paths:
    M LayoutTests/TestExpectations
    A LayoutTests/fast/inline/line-clamp-with-max-height-overflow-expected.html
    A LayoutTests/fast/inline/line-clamp-with-max-height-overflow.html
    M LayoutTests/platform/ios/fast/overflow/line-clamp-expected.txt
    M LayoutTests/platform/mac/fast/overflow/line-clamp-expected.txt
    M Source/WebCore/rendering/RenderBlockFlow.cpp

  Log Message:
  -----------
  [IFC][line-clamp](REGRESSION 264048 at main) Yahoo.com: Words are cutoff on article titles
https://bugs.webkit.org/show_bug.cgi?id=257629
<rdar://110018946>

Reviewed by Simon Fraser.

1. The "cutoff article titles" is some visible inline content _after_ the clamped line. They are supposed to be clipped off.
2. The reason why they are visible is because we compute incorrect logical height value for the flex container (box with -webkit-line-clamp)
3. This incorrectly computed height value makes the flex container too tall revealing content _after_ the clamped line.

The input to the flex container height computation is the flex items' accumulated height (bottom of the last flex item).
Normally this is based on the _clamped_ content height (see the end of RenderDeprecatedFlexibleBox::layoutVerticalBox), but apparently 264048 at main did not cover all the cases.

In 264048 at main we started tracking both clamped and unclamped content height on each flex items
1. clamped height is used to compute the flex container's final height
2. unclamped height is used to prevent sibling content from getting overlapped

This (#2) was supposed to provide a more reasonable -webkit-line-clamp result; see an example below:

 <div style="display: flex; -webkit-line-clamp: 1;">
   <div flex-item>
     flex item is clamped here (this line ends with ellipsis)
     this line is still visible. It is expected unless overflow clipping is applied.
   </div>
   <div flex-item>
     this sibling content is visible and prior to 264048 at main it overlapped the lines in the first flex-item
     as the first flex-item height is set to the clamped content height.
   </div>
 </div>

Starting from 264048 at main, the second flex-item is placed _below_ the first flex item (and not overlap it)
This was achieved by letting the flex items keep their unclamped logical height.

However some part of the RenderDeprecatedFlexibleBox (flex container) expects the flex items' logical height match their clamped content height (this is also legacy behavior).

In this patch we start setting the _clamped_ content height on the flex items.

e.g. (assume 20px as line box height)
 <div flex-container with -webkit-line-clamp: 1>
   <div flex-item>
     Flex item is clamped here (this line ends with ellipsis)
     but this line is still visible which is expected unless overflow is clipping is applied.
   </div>
 </div>

The used height of the flex-item is now the height of the first line (20px) (as opposed to the bottom of the second line (40px))

* LayoutTests/TestExpectations:
* LayoutTests/fast/inline/line-clamp-with-max-height-overflow-expected.html: Added.
* LayoutTests/fast/inline/line-clamp-with-max-height-overflow.html: Added.
* LayoutTests/platform/ios/fast/overflow/line-clamp-expected.txt:
* LayoutTests/platform/mac/fast/overflow/line-clamp-expected.txt:
* Source/WebCore/rendering/RenderBlockFlow.cpp:
(WebCore::RenderBlockFlow::layoutModernLines): The only functional change here is:
 setLogicalHeight(borderAndPaddingBefore() + *logicalHeight + borderAndPaddingAfter() + scrollbarLogicalHeight());
where we set the clamped content height as the logical height.

Canonical link: https://commits.webkit.org/264828@main




More information about the webkit-changes mailing list