[webkit-reviews] review granted: [Bug 185982] iOS: setting 'defaultValue' of input type=date from script does not cause UI update : [Attachment 341301] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Stephen McGruer
<smcgruer at chromium.org>'s request for review:
Bug 185982: iOS: setting 'defaultValue' of input type=date from script does not
cause UI update
https://bugs.webkit.org/show_bug.cgi?id=185982

Attachment 341301: Patch

https://bugs.webkit.org/attachment.cgi?id=341301&action=review




--- Comment #4 from Darin Adler <darin at apple.com> ---
Comment on attachment 341301
  --> https://bugs.webkit.org/attachment.cgi?id=341301
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.