Bug 222013

Summary: [iOS][FCR] Add new look for input type=date
Product: WebKit Reporter: Aditya Keerthi <akeerthi>
Component: FormsAssignee: Aditya Keerthi <akeerthi>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, changseok, darin, esprehn+autocc, ews-watchlist, glenn, gyuyoung.kim, kondapallykalyan, macpherson, menard, pdr, webkit-bug-importer, wenson_hsieh
Priority: P2 Keywords: InRadar
Version: Safari Technology Preview   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

Aditya Keerthi
Reported 2021-02-16 17:52:33 PST
...
Attachments
Patch (3.12 KB, patch)
2021-02-16 17:55 PST, Aditya Keerthi
no flags
Patch (3.12 KB, patch)
2021-02-17 07:39 PST, Aditya Keerthi
no flags
Aditya Keerthi
Comment 1 2021-02-16 17:52:51 PST
Aditya Keerthi
Comment 2 2021-02-16 17:55:11 PST
Aditya Keerthi
Comment 3 2021-02-17 07:39:57 PST
Aditya Keerthi
Comment 4 2021-02-17 10:56:26 PST
Comment on attachment 420645 [details] Patch Thanks for the review!
EWS
Comment 5 2021-02-17 11:01:25 PST
Committed r273014: <https://commits.webkit.org/r273014> All reviewed patches have been landed. Closing bug and clearing flags on attachment 420645 [details].
Darin Adler
Comment 6 2021-02-17 13:08:57 PST
Comment on attachment 420645 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=420645&action=review > Source/WebCore/rendering/RenderThemeIOS.mm:688 > + width = static_cast<int>(std::ceil(maximumWidth)); Seems a little bit messy (not newly so). Not sure I understand the strategy behind using int here. Why would we would want to floor or ceil to an integer instead of letting rendering code snap to device pixel boundaries? And floor in the old code and ceil in the new? Length can hold LayoutUnit values, not just integers, so the int isn’t necessary, so it’s a choice.
Aditya Keerthi
Comment 7 2021-02-17 15:01:13 PST
(In reply to Darin Adler from comment #6) > Comment on attachment 420645 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=420645&action=review > > > Source/WebCore/rendering/RenderThemeIOS.mm:688 > > + width = static_cast<int>(std::ceil(maximumWidth)); > > Seems a little bit messy (not newly so). Not sure I understand the strategy > behind using int here. Why would we would want to floor or ceil to an > integer instead of letting rendering code snap to device pixel boundaries? > And floor in the old code and ceil in the new? Length can hold LayoutUnit > values, not just integers, so the int isn’t necessary, so it’s a choice. I suppose we could just use the float value rather than an int. The only reason I used an int in the new code is to match the original logic. The important thing here is that the width of the date control is just large enough to fit the date on one line. The ceil is necessary in the new code to avoid rounding down and making the width too small. The floor only works in the old code because of the additional padding added (ensuring the width is large enough). I can make the change to use the float value in a follow-up.
Note You need to log in before you can comment on or make changes to this bug.