[GTK] Datalist element support for TextFieldInputType

• Component: WebKitGTK
• Status: RESOLVED FIXED

[Zan Dobersek]

Enable the datalist element support and unskip related tests.

[ChangSeok Oh]

@Zan, Are you working on this? If you're not now, I'd like to follow this up.

[Zan Dobersek]

Nope, feel free to hack on this.

[ChangSeok Oh]

Here is a rough sketch. http://cgit.collabora.com/git/user/kevino/WebKit.git/log/?h=datalist

I'm going to start upstream through several split bugs.

[ChangSeok Oh]

Created attachment 218910 [details]
Patch

[WebKit Commit Bot]

Attachment 218910 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'ChangeLog', u'Source/WebCore/ChangeLog', u'Source/WebCore/GNUmakefile.am', u'Source/WebCore/GNUmakefile.list.am', u'Source/WebCore/css/themeGtk.css', u'Source/WebCore/page/Page.cpp', u'Source/WebCore/page/Page.h', u'Source/WebCore/page/SuggestionBoxController.cpp', u'Source/WebCore/page/SuggestionBoxController.h', u'Source/WebCore/page/SuggestionElement.cpp', u'Source/WebCore/page/SuggestionElement.h', u'Source/WebCore/platform/gtk/RenderThemeGtk.cpp', u'Source/WebCore/platform/gtk/RenderThemeGtk.h', u'Source/WebKit2/ChangeLog', u'Source/WebKit2/WebProcess/InjectedBundle/API/gtk/WebKitWebPage.cpp', u'Source/autotools/PrintBuildConfiguration.m4', u'Source/autotools/ReadCommandLineArguments.m4', u'Source/autotools/SetupAutomake.m4', u'Source/autotools/SetupWebKitFeatures.m4', '--commit-queue']" exit_code: 1
ERROR: Source/WebCore/page/SuggestionElement.cpp:35:  Comma should be at the beginning of the line in a member initialization list.  [whitespace/init] [4]
Total errors found: 1 in 19 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[EFL EWS Bot]

Comment on attachment 218910 [details]
Patch

Attachment 218910 [details] did not pass efl-ews (efl):
Output: http://webkit-queues.appspot.com/results/47258329

[EFL EWS Bot]

Comment on attachment 218910 [details]
Patch

Attachment 218910 [details] did not pass efl-wk2-ews (efl-wk2):
Output: http://webkit-queues.appspot.com/results/47218401

[ChangSeok Oh]

Created attachment 218947 [details]
Patch

[WebKit Commit Bot]

Attachment 218947 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'ChangeLog', u'Source/WebCore/CMakeLists.txt', u'Source/WebCore/ChangeLog', u'Source/WebCore/GNUmakefile.am', u'Source/WebCore/GNUmakefile.list.am', u'Source/WebCore/css/themeGtk.css', u'Source/WebCore/page/Page.cpp', u'Source/WebCore/page/Page.h', u'Source/WebCore/page/SuggestionBoxController.cpp', u'Source/WebCore/page/SuggestionBoxController.h', u'Source/WebCore/page/SuggestionElement.cpp', u'Source/WebCore/page/SuggestionElement.h', u'Source/WebCore/platform/gtk/RenderThemeGtk.cpp', u'Source/WebCore/platform/gtk/RenderThemeGtk.h', u'Source/WebKit2/ChangeLog', u'Source/WebKit2/WebProcess/InjectedBundle/API/gtk/WebKitWebPage.cpp', u'Source/autotools/PrintBuildConfiguration.m4', u'Source/autotools/ReadCommandLineArguments.m4', u'Source/autotools/SetupAutomake.m4', u'Source/autotools/SetupWebKitFeatures.m4', '--commit-queue']" exit_code: 1
ERROR: Source/WebCore/page/SuggestionElement.cpp:35:  Comma should be at the beginning of the line in a member initialization list.  [whitespace/init] [4]
Total errors found: 1 in 20 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[ChangSeok Oh]

(In reply to comment #9)
> Attachment 218947 [details] did not pass style-queue:
> 
> Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'ChangeLog', u'Source/WebCore/CMakeLists.txt', u'Source/WebCore/ChangeLog', u'Source/WebCore/GNUmakefile.am', u'Source/WebCore/GNUmakefile.list.am', u'Source/WebCore/css/themeGtk.css', u'Source/WebCore/page/Page.cpp', u'Source/WebCore/page/Page.h', u'Source/WebCore/page/SuggestionBoxController.cpp', u'Source/WebCore/page/SuggestionBoxController.h', u'Source/WebCore/page/SuggestionElement.cpp', u'Source/WebCore/page/SuggestionElement.h', u'Source/WebCore/platform/gtk/RenderThemeGtk.cpp', u'Source/WebCore/platform/gtk/RenderThemeGtk.h', u'Source/WebKit2/ChangeLog', u'Source/WebKit2/WebProcess/InjectedBundle/API/gtk/WebKitWebPage.cpp', u'Source/autotools/PrintBuildConfiguration.m4', u'Source/autotools/ReadCommandLineArguments.m4', u'Source/autotools/SetupAutomake.m4', u'Source/autotools/SetupWebKitFeatures.m4', '--commit-queue']" exit_code: 1
> ERROR: Source/WebCore/page/SuggestionElement.cpp:35:  Comma should be at the beginning of the line in a member initialization list.  [whitespace/init] [4]
> Total errors found: 1 in 20 files
> 
> 
> If any of these errors are false positives, please file a bug against check-webkit-style.

Yeah.. I think this is a false alarm

[EFL EWS Bot]

Comment on attachment 218947 [details]
Patch

Attachment 218947 [details] did not pass efl-wk2-ews (efl-wk2):
Output: http://webkit-queues.appspot.com/results/47338279

[ChangSeok Oh]

(In reply to comment #11)
> (From update of attachment 218947 [details])
> Attachment 218947 [details] did not pass efl-wk2-ews (efl-wk2):
> Output: http://webkit-queues.appspot.com/results/47338279

I think this is an another false alarm. I couldn't find any build issue with this change on efl port. What happens on the EFL bot.. :(

[ChangSeok Oh]

Comment on attachment 218947 [details]
Patch

I got a better idea using shadow html like media controls.

[ChangSeok Oh]

Created attachment 223347 [details]
Patch

[ChangSeok Oh]

Created attachment 223351 [details]
Patch

[ChangSeok Oh]

Created attachment 223431 [details]
Add a test to verify the functionality

[ChangSeok Oh]

Created attachment 223433 [details]
Patch

[Joacim Löwgren]

Anything happening on this?

[Michael Catanzaro]

Comment on attachment 223433 [details]
Patch

Hi,

Apologies that your patch was not reviewed in a timely manner. Since it's now quite old, I am removing it from the review request queue. Please consider rebasing it on trunk and resubmitting.

To increase the chances of getting a review, consider using 'Tools/Scripts/webkit-patch upload --suggest-reviewers' to CC reviewers who might be interested in this bug.

[Mattia Astorino]

Any update about this? Edge, Chrome and Firefox supports the <datalist> element..

[Teun]

It appears that support for datalist has been added to Safari?
Tested in 12.1.1 and it works!

[Michael Catanzaro]

Yes, but this is a GTK bug and it doesn't work for GTK yet.

[Carlos Garcia Campos]

Created attachment 375155 [details]
Patch

[Carlos Garcia Campos]

Created attachment 375156 [details]
Patch

[Michael Catanzaro]

Comment on attachment 375156 [details]
Patch

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

> Source/WebKit/UIProcess/gtk/WebDataListSuggestionsDropdownGtk.cpp:51
> +    g_signal_connect(treeView, "row-activated", G_CALLBACK(treeViewRowActivatedCallback), this);

I'm nervous about this one. I see it isn't disconnected in the destructor. Surely it should use g_signal_connect_object()? Even if you always expect it to be disconnected in the callback, better safe than sorry?

[Carlos Garcia Campos]

Comment on attachment 375156 [details]
Patch

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

>> Source/WebKit/UIProcess/gtk/WebDataListSuggestionsDropdownGtk.cpp:51
>> +    g_signal_connect(treeView, "row-activated", G_CALLBACK(treeViewRowActivatedCallback), this);
> 
> I'm nervous about this one. I see it isn't disconnected in the destructor. Surely it should use g_signal_connect_object()? Even if you always expect it to be disconnected in the callback, better safe than sorry?

We can't use connect_object because this is not a GObject. The treeview is destroyed in the destructor, so it's not possible that row activated is emitted after this has been deleted.

[Michael Catanzaro]

Comment on attachment 375156 [details]
Patch

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

>>> Source/WebKit/UIProcess/gtk/WebDataListSuggestionsDropdownGtk.cpp:51
>>> +    g_signal_connect(treeView, "row-activated", G_CALLBACK(treeViewRowActivatedCallback), this);
>> 
>> I'm nervous about this one. I see it isn't disconnected in the destructor. Surely it should use g_signal_connect_object()? Even if you always expect it to be disconnected in the callback, better safe than sorry?
> 
> We can't use connect_object because this is not a GObject. The treeview is destroyed in the destructor, so it's not possible that row activated is emitted after this has been deleted.

OK except the treeview is not actually destroyed in the destructor? I would disconnect manually in the destructor.

[Carlos Garcia Campos]

(In reply to Michael Catanzaro from comment #27)
> Comment on attachment 375156 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=375156&action=review
> 
> >>> Source/WebKit/UIProcess/gtk/WebDataListSuggestionsDropdownGtk.cpp:51
> >>> +    g_signal_connect(treeView, "row-activated", G_CALLBACK(treeViewRowActivatedCallback), this);
> >> 
> >> I'm nervous about this one. I see it isn't disconnected in the destructor. Surely it should use g_signal_connect_object()? Even if you always expect it to be disconnected in the callback, better safe than sorry?
> > 
> > We can't use connect_object because this is not a GObject. The treeview is destroyed in the destructor, so it's not possible that row activated is emitted after this has been deleted.
> 
> OK except the treeview is not actually destroyed in the destructor? I would
> disconnect manually in the destructor.

The treeview is a children of the popup window, when a widget is destroyed, all its children are destroyed too.

[Carlos Garcia Campos]

Committed r248033: <https://trac.webkit.org/changeset/248033>