input.showPicker() should throw when input is readonly/disabled

• Component: DOM
• Status: RESOLVED FIXED

[Tim Nguyen (:ntim)]

See: html/semantics/forms/the-input-element/show-picker-disabled-readonly.html

[Radar WebKit Bug Importer]

<rdar://problem/93090068>

[Tim Nguyen (:ntim)]

https://chromium-review.googlesource.com/c/chromium/src/+/3563070/6/third_party/blink/renderer/core/html/forms/html_input_element.cc

Created attachment 459223 [details]
Patch

The patch needs rebase once the patch for bug 240301 is landed.

[EWS Watchlist]

This patch modifies the imported WPT tests. Please ensure that any changes on the tests (not coming from a WPT import) are exported to WPT. Please see https://trac.webkit.org/wiki/WPTExportProcess

[Tim Nguyen (:ntim)]

Comment on attachment 459223 [details]
Patch

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

> LayoutTests/imported/w3c/web-platform-tests/html/semantics/forms/the-input-element/show-picker-disabled-readonly-expected.txt:40
> +FAIL input[type=file] showPicker() doesn't throw when readonly assert_throws_dom: function "() => { input.showPicker(); }" threw object "InvalidStateError: Input showPicker cannot be used on immutable controls." that is not a DOMException NotAllowedError: property "code" is equal to 11, expected 0
> +FAIL input[type=hidden] showPicker() doesn't throw when readonly assert_throws_dom: function "() => { input.showPicker(); }" threw object "InvalidStateError: Input showPicker cannot be used on immutable controls." that is not a DOMException NotAllowedError: property "code" is equal to 11, expected 0
> +FAIL input[type=image] showPicker() doesn't throw when readonly assert_throws_dom: function "() => { input.showPicker(); }" threw object "InvalidStateError: Input showPicker cannot be used on immutable controls." that is not a DOMException NotAllowedError: property "code" is equal to 11, expected 0
> +PASS input[type=month] showPicker() throws when readonly
> +PASS input[type=number] showPicker() throws when readonly
> +PASS input[type=password] showPicker() throws when readonly
> +FAIL input[type=radio] showPicker() doesn't throw when readonly assert_throws_dom: function "() => { input.showPicker(); }" threw object "InvalidStateError: Input showPicker cannot be used on immutable controls." that is not a DOMException NotAllowedError: property "code" is equal to 11, expected 0
> +FAIL input[type=range] showPicker() doesn't throw when readonly assert_throws_dom: function "() => { input.showPicker(); }" threw object "InvalidStateError: Input showPicker cannot be used on immutable controls." that is not a DOMException NotAllowedError: property "code" is equal to 11, expected 0
> +FAIL input[type=reset] showPicker() doesn't throw when readonly assert_throws_dom: function "() => { input.showPicker(); }" threw object "InvalidStateError: Input showPicker cannot be used on immutable controls." that is not a DOMException NotAllowedError: property "code" is equal to 11, expected 0
> +PASS input[type=search] showPicker() throws when readonly
> +FAIL input[type=submit] showPicker() doesn't throw when readonly assert_throws_dom: function "() => { input.showPicker(); }" threw object "InvalidStateError: Input showPicker cannot be used on immutable controls." that is not a DOMException NotAllowedError: property "code" is equal to 11, expected 0

The different fails here are a regression (see: https://wpt.fyi/results/html/semantics/forms/the-input-element/show-picker-disabled-readonly.html?label=experimental&label=master&aligned) 

Seems like the test expects a different error for readonly, based on the input type.

Firefox's implementation might be relevant here: https://phabricator.services.mozilla.com/D142754#change-NirQH5Dh8sEo, since it's the only one implementing this correctly.

Seems like the spec is consistent with Firefox as well: https://html.spec.whatwg.org/multipage/input.html#dom-input-showpicker

[Tim Nguyen (:ntim)]

I found the more exact bit that describes on which elements the readonly attribute applies:

https://html.spec.whatwg.org/multipage/input.html#do-not-apply

[Tim Nguyen (:ntim)]

Comment on attachment 459223 [details]
Patch

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

r=me with the commit message fixed.

> Source/WebCore/ChangeLog:8
> +        input.showPicker() should throw when input is readonly/disabled on supported types
> +        https://bugs.webkit.org/show_bug.cgi?id=240302
> +
> +        Reviewed by NOBODY (OOPS!).
> +
> +        When input is readonly or disabled on supported types, input.showPicker() should throw InvalidStateError.

Can you remove the "on supported types" bits? 

I filed bug 240343 to fix the readonly supported types.

[Tim Nguyen (:ntim)]

Comment on attachment 459223 [details]
Patch

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

> Source/WebCore/html/HTMLInputElement.cpp:1255
> +        return Exception { InvalidStateError, "Input showPicker cannot be used on immutable controls."_s };

showPicker() to be consistent with other messages.

Created attachment 459230 [details]
Patch

Created attachment 459231 [details]
Patch

[Tim Nguyen (:ntim)]

Comment on attachment 459231 [details]
Patch

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

> Source/WebCore/ChangeLog:3
> +        input.showPicker() should throw when input is readonly/disabled on supported types

The title needs updating too, both in LayoutTests/imported/w3c/ChangeLog and Source/WebCore/ChangeLog

Created attachment 459233 [details]
Patch

[Tim Nguyen (:ntim)]

Looks like WPT expectations need to be updated with the new message

Created attachment 459297 [details]
[fast-cq] Patch

[EWS]

Committed r294163 (250532@main): <https://commits.webkit.org/250532@main>

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

*** Bug 239302 has been marked as a duplicate of this bug. ***