Bug 216005

Created attachment 407686 [details] Patch

Comment on attachment 407686 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=407686&action=review > Source/WebCore/html/shadow/DateTimeFieldElement.cpp:64 > + if (!isFieldOwnerDisabled() && !isFieldOwnerReadOnly()) { `readonly` inputs are still able to receive events. They basically are the same as regular inputs except that typing doesn't update the `value`. Is that still possible with this? Am I reading this code wrong?

(In reply to Devin Rousso from comment #2) > Comment on attachment 407686 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=407686&action=review > > > Source/WebCore/html/shadow/DateTimeFieldElement.cpp:64 > > + if (!isFieldOwnerDisabled() && !isFieldOwnerReadOnly()) { > > `readonly` inputs are still able to receive events. They basically are the > same as regular inputs except that typing doesn't update the `value`. Is > that still possible with this? Am I reading this code wrong? Keyboard events are still dispatched on the element. This code just prevents editability – `handleKeyboardEvent` is a virtual method that the editable fields use to respond to keyboard events.

Comment on attachment 407686 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=407686&action=review r=me, nice works! > Source/WebCore/css/html.css:520 > +input[disabled]::-webkit-datetime-edit-year-field, NIT: should these be `input[type="date"]`? > Source/WebCore/html/BaseChooserOnlyDateAndTimeInputType.cpp:318 > + return element()->isDisabledFormControl(); NIT: should this have `ASSERT(element())` too? > Source/WebCore/html/shadow/DateTimeFieldElement.cpp:119 > +bool DateTimeFieldElement::isFocusable() const (for my own knowledge) is this what's used to determine whether or not the `<input type="date">` is mouse-interactable? I ask because `disabled` should prevent all interaction but `readonly` should still allow mouse interaction.

Created attachment 407818 [details] Patch for landing

(In reply to Devin Rousso from comment #4) > Comment on attachment 407686 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=407686&action=review > > r=me, nice works! Thanks for the review! > > Source/WebCore/css/html.css:520 > > +input[disabled]::-webkit-datetime-edit-year-field, > > NIT: should these be `input[type="date"]`? They probably should. I've added a FIXME to address this in a follow-up, since there are existing instances that should also be updated. > > Source/WebCore/html/BaseChooserOnlyDateAndTimeInputType.cpp:318 > > + return element()->isDisabledFormControl(); > > NIT: should this have `ASSERT(element())` too? Added an assertion. > > Source/WebCore/html/shadow/DateTimeFieldElement.cpp:119 > > +bool DateTimeFieldElement::isFocusable() const > > (for my own knowledge) is this what's used to determine whether or not the > `<input type="date">` is mouse-interactable? I ask because `disabled` > should prevent all interaction but `readonly` should still allow mouse > interaction. Partially. If by mouse-interaction, you mean the ability to select the individual components of the control using a mouse, the answer is yes. If by mouse-interaction, you mean the dispatching of mouse events, that is handled in existing code in Node::handleLocalEvents, which does not dispatch events if Element::isDisabledFormControl is true.

Committed r266514: <https://trac.webkit.org/changeset/266514> All reviewed patches have been landed. Closing bug and clearing flags on attachment 407818 [details].

<rdar://problem/68278191>