Bug 221350 - [macOS] Selecting a date on datetime-local inputs unexpectedly adds second and millisecond fields
Summary: [macOS] Selecting a date on datetime-local inputs unexpectedly adds second an...
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-03 12:09 PST by Aditya Keerthi
Modified: 2021-02-04 06:59 PST (History)
9 users (show)

See Also:


Attachments
Patch (27.47 KB, patch)
2021-02-03 12:12 PST, Aditya Keerthi
drousso: review+
Details | Formatted Diff | Diff
Patch for landing (27.69 KB, patch)
2021-02-03 13:53 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-03 12:09:41 PST
Steps To Reproduce:
1. Go to https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input#input_types
2. Scroll down to the “datetime-local” input
3. Click on the input to open the calendar
4. Click a date on the calendar

Results:
The second and millisecond fields are added to the input after choosing a date from the calendar. These fields should not be added to the input.
Comment 1 Aditya Keerthi 2021-02-03 12:09:58 PST
<rdar://problem/73943517>
Comment 2 Aditya Keerthi 2021-02-03 12:12:55 PST
Created attachment 419170 [details]
Patch
Comment 3 Devin Rousso 2021-02-03 13:18:36 PST
Comment on attachment 419170 [details]
Patch

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

r=me, nice patch!

> Source/WebCore/html/BaseDateAndTimeInputType.cpp:548
> +    auto date = parseToDateComponents(element.value());
> +    if (date) {

rather than have an `else` could you `parseToDateComponents(element.value()).valueOr({ })`?

> Source/WebCore/platform/DateTimeChooserParameters.h:54
>      bool required;
>      bool isAnchorElementRTL;
>      bool useDarkAppearance;
> +    bool hasSecondField;
> +    bool hasMillisecondField;

I wonder if all of these should have a default value `{ false }` (or `{ 0 }` for the `double`s) or something 🤔
Comment 4 Aditya Keerthi 2021-02-03 13:34:45 PST
(In reply to Devin Rousso from comment #3)
> Comment on attachment 419170 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=419170&action=review
> 
> r=me, nice patch!

Thanks for the review!
 
> > Source/WebCore/html/BaseDateAndTimeInputType.cpp:548
> > +    auto date = parseToDateComponents(element.value());
> > +    if (date) {
> 
> rather than have an `else` could you
> `parseToDateComponents(element.value()).valueOr({ })`?

Didn't know about valueOr! Will make the change.
 
> > Source/WebCore/platform/DateTimeChooserParameters.h:54
> >      bool required;
> >      bool isAnchorElementRTL;
> >      bool useDarkAppearance;
> > +    bool hasSecondField;
> > +    bool hasMillisecondField;
> 
> I wonder if all of these should have a default value `{ false }` (or `{ 0 }`
> for the `double`s) or something 🤔

They probably should, will make the change.
Comment 5 Aditya Keerthi 2021-02-03 13:53:46 PST
Created attachment 419185 [details]
Patch for landing
Comment 6 EWS 2021-02-04 06:59:29 PST
Committed r272368: <https://trac.webkit.org/changeset/272368>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 419185 [details].