Bug 240302 - input.showPicker() should throw when input is readonly/disabled
Summary: input.showPicker() should throw when input is readonly/disabled
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: Safari 14
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: zsun
URL:
Keywords: InRadar, WPTImpact
Depends on: 240301
Blocks:
  Show dependency treegraph
 
Reported: 2022-05-11 04:38 PDT by Tim Nguyen (:ntim)
Modified: 2022-05-13 10:26 PDT (History)
10 users (show)

See Also:


Attachments
Patch (10.05 KB, patch)
2022-05-12 04:58 PDT, zsun
no flags Details | Formatted Diff | Diff
Patch (10.13 KB, patch)
2022-05-12 07:13 PDT, zsun
no flags Details | Formatted Diff | Diff
Patch (10.13 KB, patch)
2022-05-12 07:13 PDT, zsun
no flags Details | Formatted Diff | Diff
Patch (10.09 KB, patch)
2022-05-12 07:49 PDT, zsun
no flags Details | Formatted Diff | Diff
[fast-cq] Patch (18.09 KB, patch)
2022-05-13 06:19 PDT, zsun
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tim Nguyen (:ntim) 2022-05-11 04:38:32 PDT
See: html/semantics/forms/the-input-element/show-picker-disabled-readonly.html
Comment 1 Radar WebKit Bug Importer 2022-05-11 04:38:39 PDT
<rdar://problem/93090068>
Comment 3 zsun 2022-05-12 04:58:43 PDT
Created attachment 459223 [details]
Patch
Comment 4 zsun 2022-05-12 04:59:44 PDT
The patch needs rebase once the patch for bug 240301 is landed.
Comment 5 EWS Watchlist 2022-05-12 05:02:38 PDT
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
Comment 6 Tim Nguyen (:ntim) 2022-05-12 05:59:09 PDT Comment hidden (obsolete)
Comment 7 Tim Nguyen (:ntim) 2022-05-12 06:45:49 PDT
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
Comment 8 Tim Nguyen (:ntim) 2022-05-12 06:55:05 PDT
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.
Comment 9 Tim Nguyen (:ntim) 2022-05-12 06:56:11 PDT
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.
Comment 10 zsun 2022-05-12 07:13:01 PDT
Created attachment 459230 [details]
Patch
Comment 11 zsun 2022-05-12 07:13:56 PDT
Created attachment 459231 [details]
Patch
Comment 12 Tim Nguyen (:ntim) 2022-05-12 07:44:39 PDT
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
Comment 13 zsun 2022-05-12 07:49:22 PDT
Created attachment 459233 [details]
Patch
Comment 14 Tim Nguyen (:ntim) 2022-05-12 09:28:03 PDT
Looks like WPT expectations need to be updated with the new message
Comment 15 zsun 2022-05-13 06:19:18 PDT
Created attachment 459297 [details]
[fast-cq] Patch
Comment 16 EWS 2022-05-13 10:26:25 PDT
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].