Bug 222013 - [iOS][FCR] Add new look for input type=date
Summary: [iOS][FCR] Add new look for input type=date
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Forms (show other bugs)
Version: Safari Technology Preview
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Aditya Keerthi
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-02-16 17:52 PST by Aditya Keerthi
Modified: 2021-02-17 15:01 PST (History)
13 users (show)

See Also:


Attachments
Patch (3.12 KB, patch)
2021-02-16 17:55 PST, Aditya Keerthi
no flags Details | Formatted Diff | Diff
Patch (3.12 KB, patch)
2021-02-17 07:39 PST, Aditya Keerthi
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Aditya Keerthi 2021-02-16 17:52:33 PST
...
Comment 1 Aditya Keerthi 2021-02-16 17:52:51 PST
<rdar://problem/74415329>
Comment 2 Aditya Keerthi 2021-02-16 17:55:11 PST
Created attachment 420569 [details]
Patch
Comment 3 Aditya Keerthi 2021-02-17 07:39:57 PST
Created attachment 420645 [details]
Patch
Comment 4 Aditya Keerthi 2021-02-17 10:56:26 PST
Comment on attachment 420645 [details]
Patch

Thanks for the review!
Comment 5 EWS 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].
Comment 6 Darin Adler 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.
Comment 7 Aditya Keerthi 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.