Bug 199806

Summary: [Text autosizing] [iPadOS] Product label text is clipped in portrait mode on the front page of sephora.com
Product: WebKit Reporter: Wenson Hsieh <wenson_hsieh>
Component: TextAssignee: Wenson Hsieh <wenson_hsieh>
Status: RESOLVED FIXED    
Severity: Normal CC: bdakin, commit-queue, mmaxfield, thorton, webkit-bug-importer, zalan
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
zalan: review+
Waiting for EWS
none
Actually add test expectation changes (oops) none

Wenson Hsieh
Reported 2019-07-15 14:05:26 PDT
<rdar://problem/52902482>
Attachments
Patch (7.35 KB, patch)
2019-07-15 16:40 PDT, Wenson Hsieh 		zalan: review+
DetailsFormatted DiffDiff
Waiting for EWS (7.31 KB, patch)
2019-07-15 17:07 PDT, Wenson Hsieh 		no flags
DetailsFormatted DiffDiff
Actually add test expectation changes (oops) (8.15 KB, patch)
2019-07-15 17:26 PDT, Wenson Hsieh 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Wenson Hsieh
Comment 1 2019-07-15 16:40:41 PDT
Created attachment 374165 [details] Patch
zalan
Comment 2 2019-07-15 16:49:30 PDT
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 3 2019-07-15 16:53:05 PDT
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
Comment 4 2019-07-15 17:07:41 PDT
Created attachment 374167 [details] Waiting for EWS
Wenson Hsieh
Comment 5 2019-07-15 17:26:32 PDT
Created attachment 374170 [details] Actually add test expectation changes (oops)
WebKit Commit Bot
Comment 6 2019-07-15 19:51:28 PDT
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
Comment 7 2019-07-15 19:51:29 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.