[Text autosizing] [iPadOS] Product label text is clipped in portrait mode on the front page of sephora.com

[Wenson Hsieh]

<rdar://problem/52902482>

[Wenson Hsieh]

Created attachment 374165 [details]
Patch

[zalan]

Comment on attachment 374165 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=374165&action=review

> Source/WebCore/rendering/style/TextSizeAdjustment.cpp:56
> +        Length heightOrMaxHeightAsLength;

Optional<Length> instead of checking Length.isAuto() might reflect the logic better.

> Source/WebCore/rendering/style/TextSizeAdjustment.cpp:74
> +        return approximateNumberOfLines < 6 && approximateNumberOfLines - std::floor(approximateNumberOfLines) < 0.01;

Could you name the constant values please? It improves code readability (and works as comment too).

[Wenson Hsieh]

Comment on attachment 374165 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=374165&action=review

>> Source/WebCore/rendering/style/TextSizeAdjustment.cpp:56
>> +        Length heightOrMaxHeightAsLength;
> 
> Optional<Length> instead of checking Length.isAuto() might reflect the logic better.

Good call! Will change to Optional<Length>

>> Source/WebCore/rendering/style/TextSizeAdjustment.cpp:74
>> +        return approximateNumberOfLines < 6 && approximateNumberOfLines - std::floor(approximateNumberOfLines) < 0.01;
> 
> Could you name the constant values please? It improves code readability (and works as comment too).

Sounds good — will name the constants and remove the big comment (or just make it smaller).

[Wenson Hsieh]

Created attachment 374167 [details]
Waiting for EWS

[Wenson Hsieh]

Created attachment 374170 [details]
Actually add test expectation changes (oops)

[WebKit Commit Bot]

Comment on attachment 374170 [details]
Actually add test expectation changes (oops)

Clearing flags on attachment: 374170

Committed r247467: <https://trac.webkit.org/changeset/247467>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.