Bug 224924

Summary:

[GTK] Add picker UI for <input type=date> and <input type=datetime-local>

Product:

WebKit

Reporter:

Martin Robinson <mrobinson>

Component:

WebKitGTK

Assignee:

Martin Robinson <mrobinson>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

aperez, berto, bugs-noreply, cgarcia, ews-watchlist, gustavo

Priority:

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

Bug Blocks:

98936

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	none
Patch	none

Martin Robinson

Reported 2021-04-22 02:32:13 PDT

This bug tracks adding the dialog-driven picker UI for these input types. Without them the entries can only be modified via the keyboard.

Attachments
Patch (3.75 KB, patch) 2021-05-06 05:19 PDT, Martin Robinson	no flags	Details Formatted Diff Diff
Patch (10.81 KB, patch) 2021-05-07 02:27 PDT, Carlos Garcia Campos	no flags	Details Formatted Diff Diff
Patch (10.99 KB, patch) 2021-05-08 03:48 PDT, Carlos Garcia Campos	no flags	Details Formatted Diff Diff
Patch (10.99 KB, patch) 2021-05-08 04:01 PDT, Carlos Garcia Campos	no flags	Details Formatted Diff Diff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.

Martin Robinson

Comment 1 2021-05-06 05:19:15 PDT

Created attachment 427875 [details] Patch

EWS Watchlist

Comment 2 2021-05-06 05:20:51 PDT

Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See https://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API

Carlos Garcia Campos

Comment 3 2021-05-06 06:31:10 PDT

Comment on attachment 427875 [details] Patch This is wip, let's remove the r? flag.

Carlos Garcia Campos

Comment 4 2021-05-07 02:27:29 PDT

Created attachment 427980 [details] Patch

Martin Robinson

Comment 5 2021-05-07 06:30:54 PDT

Comment on attachment 427980 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=427980&action=review > Source/WebKit/UIProcess/gtk/WebDateTimePickerGtk.cpp:70 > + webkitWebViewBaseSetShouldNotifyFocusEvents(WEBKIT_WEB_VIEW_BASE(webView), false); Is it possible that you meant to write: webkitWebViewBaseSetShouldNotifyFocusEvents(WEBKIT_WEB_VIEW_BASE(webView), true); here?

Carlos Garcia Campos

Comment 6 2021-05-07 07:56:10 PDT

Comment on attachment 427980 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=427980&action=review >> Source/WebKit/UIProcess/gtk/WebDateTimePickerGtk.cpp:70 >> + webkitWebViewBaseSetShouldNotifyFocusEvents(WEBKIT_WEB_VIEW_BASE(webView), false); > > Is it possible that you meant to write: > > webkitWebViewBaseSetShouldNotifyFocusEvents(WEBKIT_WEB_VIEW_BASE(webView), true); > > here? Indeed! Good catch.

Carlos Garcia Campos

Comment 7 2021-05-08 03:48:30 PDT

Created attachment 428084 [details] Patch

Carlos Garcia Campos

Comment 8 2021-05-08 04:01:16 PDT

Created attachment 428085 [details] Patch

Adrian Perez

Comment 9 2021-05-10 03:12:41 PDT

Comment on attachment 428085 [details] Patch Patch LGTM, with a small nit to take into account before landing. Thanks! View in context: https://bugs.webkit.org/attachment.cgi?id=428085&action=review > Source/WebKit/UIProcess/API/gtk/WebKitWebViewBasePrivate.h:118 > +void webkitWebViewBasePageGrabbedTouch(WebKitWebViewBase*); Why the change to add the name of the “webkitWebViewBase“ argument to “webkitWebViewBasePageGrabbedTouch()”? That seems unneeded, please make sure to only add the prototype for the new function here.

Carlos Garcia Campos

Comment 10 2021-05-10 03:21:29 PDT

Comment on attachment 428085 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=428085&action=review >> Source/WebKit/UIProcess/API/gtk/WebKitWebViewBasePrivate.h:118 >> +void webkitWebViewBasePageGrabbedTouch(WebKitWebViewBase*); > > Why the change to add the name of the “webkitWebViewBase“ argument > to “webkitWebViewBasePageGrabbedTouch()”? That seems unneeded, please > make sure to only add the prototype for the new function here. It's the opposite, it's removing the name. I missed it when reviewing the patch and noticed it when adding the new function.

Adrian Perez

Comment 11 2021-05-10 04:58:46 PDT

Comment on attachment 428085 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=428085&action=review >>> Source/WebKit/UIProcess/API/gtk/WebKitWebViewBasePrivate.h:118 >>> +void webkitWebViewBasePageGrabbedTouch(WebKitWebViewBase*); >> >> Why the change to add the name of the “webkitWebViewBase“ argument >> to “webkitWebViewBasePageGrabbedTouch()”? That seems unneeded, please >> make sure to only add the prototype for the new function here. > > It's the opposite, it's removing the name. I missed it when reviewing the patch and noticed it when adding the new function. Ouch, you're right. I've read the diff the other way around at first 🤦‍♂️️ Let's cq+ this.

EWS

Comment 12 2021-05-10 05:17:55 PDT

Committed r277262 (237529@main): <https://commits.webkit.org/237529@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 428085 [details].

Note You need to log in before you can comment on or make changes to this bug.