Bug 185982 – iOS: setting 'defaultValue' of input type=date from script does not cause UI update

• Component: Forms
• Status: RESOLVED FIXED

[Stephen McGruer]

What steps will reproduce the problem?
(1) Create an input type=date
(2) From script, set the 'defaultValue' to a value
(3) Observe the input element's rendered UI

What is the expected result?

The UI should update to show the default value (since there was no previous selection). This happens on desktop Chrome.

What happens instead?

The UI doesn't update; the date picker is blank. However if you select it, the date picker widget appears and shows that the default value is set.


See test case http://output.jsbin.com/vocunov 

Note that this comes from a React bug that they found and worked around, sadly without informing browser vendors at the time :( - https://github.com/facebook/react/issues/7233

We recently fixed this in Chromium Android (http://crbug.com/838898), and I have a similar patch (to be uploaded shortly) which fixes it in WebKit iOS.

[Stephen McGruer]

Created attachment 341301 [details]
Patch

[Stephen McGruer]

First version of the patch has been added to this bug. I am not sure how to mark a LayoutTest as needing to be run on iOS - is that automatic in WebKit?

[Chris Dumez]

(In reply to Stephen McGruer from comment #2)
> First version of the patch has been added to this bug. I am not sure how to
> mark a LayoutTest as needing to be run on iOS - is that automatic in WebKit?

if the iOS-sim bubble becomes green, then your new test is passing on iOS.

[Darin Adler]

Comment on attachment 341301 [details]
Patch

Nice to fix this.

Patch looks OK but not ideal.

There’s already a general purpose InputType::attributeChanged function, so there is no need to add the valueAttributeChanged function. I fact, we should probably consider removing the other InputType xxxAttributeChanged functions for the various specific attributes. They are redundant, bloat the HTMLInputElement::parseAttribute function and the InputType base class unnecessarily, and cause additional unneeded work for other input types.

Note that the same issue comes up with plain text input fields; for those, TextFieldInputType::attributeChanged handles it. It’s a bit untidy to do almost the same thing for the same reason for the date/time fields, but write the code differently and separately. I assume this comes about because the programmers doing the date/time code weren’t aware of the already-existing text field code. The updateAppearance function has the same purpose as the updateInnerTextValue function, but uses a different name.

From code inspection, I notice a similar but distinct problem if the lang attribute of the element is changed, or the lang attribute of an ancestor element if it’s the ancestor that happens to determine the computed language (nearest ancestor with a lang attribute). BaseDateAndTimeInputType::localizeValue will format the value based on the locale, which depends on the computed language value, and nothing will cause BaseChooserOnlyDateAndTimeInputType::updateAppearance to be called when the locale changes.

Another similar but distinct problem can occur if the document’s content language is changed by adding a <meta> element to the document with an "http-equiv" attribute in it and there are no "lang" attributes.

Would be nice if we handled all these cases properly.

[Stephen McGruer]

Thank you for the quick and thorough review Darin. Before I respond to the majority of your review, can I check - you marked the patch as 'review+', are you happy for this version of the patch to be landed and your comments left as future work (probably for others working in this area, not myself)? Or would you prefer if we attempted to resolve your comments in this same patch?

[Darin Adler]

(In reply to Stephen McGruer from comment #5)
> Thank you for the quick and thorough review Darin. Before I respond to the
> majority of your review, can I check - you marked the patch as 'review+',
> are you happy for this version of the patch to be landed and your comments
> left as future work (probably for others working in this area, not myself)?
> Or would you prefer if we attempted to resolve your comments in this same
> patch?

Either way is OK with me. That’s why I said review+.

[Darin Adler]

I think the issues with language are going to be both challenging to fix efficiently and also challenging to test in a platform-independent way.

[WebKit Commit Bot]

Comment on attachment 341301 [details]
Patch

Clearing flags on attachment: 341301

Committed r232289: <https://trac.webkit.org/changeset/232289>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/40648857>