Bug 98934 - [GTK] Datalist element support for TextFieldInputType
Summary: [GTK] Datalist element support for TextFieldInputType
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: ChangSeok Oh
URL:
Keywords: Gtk, LayoutTestFailure
Depends on: 122590 122819
Blocks: 27247
  Show dependency treegraph
 
Reported: 2012-10-10 11:44 PDT by Zan Dobersek
Modified: 2019-07-31 01:02 PDT (History)
23 users (show)

See Also:


Attachments
Patch (40.60 KB, patch)
2013-12-10 14:56 PST, ChangSeok Oh
no flags Details | Formatted Diff | Diff
Patch (41.25 KB, patch)
2013-12-11 03:19 PST, ChangSeok Oh
eflews.bot: commit-queue-
Details | Formatted Diff | Diff
Patch (38.31 KB, patch)
2014-02-06 10:55 PST, ChangSeok Oh
no flags Details | Formatted Diff | Diff
Patch (40.25 KB, patch)
2014-02-06 11:12 PST, ChangSeok Oh
no flags Details | Formatted Diff | Diff
Add a test to verify the functionality (43.74 KB, patch)
2014-02-07 00:26 PST, ChangSeok Oh
no flags Details | Formatted Diff | Diff
Patch (43.72 KB, patch)
2014-02-07 01:47 PST, ChangSeok Oh
no flags Details | Formatted Diff | Diff
Patch (27.69 KB, patch)
2019-07-30 06:04 PDT, Carlos Garcia Campos
no flags Details | Formatted Diff | Diff
Patch (28.57 KB, patch)
2019-07-30 06:12 PDT, Carlos Garcia Campos
mcatanzaro: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Zan Dobersek 2012-10-10 11:44:16 PDT
Enable the datalist element support and unskip related tests.
Comment 1 ChangSeok Oh 2013-10-03 05:38:43 PDT
@Zan, Are you working on this? If you're not now, I'd like to follow this up.
Comment 2 Zan Dobersek 2013-10-03 09:45:55 PDT
Nope, feel free to hack on this.
Comment 3 ChangSeok Oh 2013-10-10 00:51:37 PDT
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.
Comment 4 ChangSeok Oh 2013-12-10 14:56:53 PST
Created attachment 218910 [details]
Patch
Comment 5 WebKit Commit Bot 2013-12-10 14:59:06 PST
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.
Comment 6 EFL EWS Bot 2013-12-10 15:05:47 PST
Comment on attachment 218910 [details]
Patch

Attachment 218910 [details] did not pass efl-ews (efl):
Output: http://webkit-queues.appspot.com/results/47258329
Comment 7 EFL EWS Bot 2013-12-10 19:48:16 PST
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
Comment 8 ChangSeok Oh 2013-12-11 03:19:16 PST
Created attachment 218947 [details]
Patch
Comment 9 WebKit Commit Bot 2013-12-11 03:21:28 PST
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.
Comment 10 ChangSeok Oh 2013-12-11 03:32:39 PST
(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
Comment 11 EFL EWS Bot 2013-12-11 03:37:54 PST
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
Comment 12 ChangSeok Oh 2013-12-11 05:08:27 PST
(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.. :(
Comment 13 ChangSeok Oh 2014-01-17 01:32:00 PST
Comment on attachment 218947 [details]
Patch

I got a better idea using shadow html like media controls.
Comment 14 ChangSeok Oh 2014-02-06 10:55:07 PST
Created attachment 223347 [details]
Patch
Comment 15 ChangSeok Oh 2014-02-06 11:12:56 PST
Created attachment 223351 [details]
Patch
Comment 16 ChangSeok Oh 2014-02-07 00:26:12 PST
Created attachment 223431 [details]
Add a test to verify the functionality
Comment 17 ChangSeok Oh 2014-02-07 01:47:14 PST
Created attachment 223433 [details]
Patch
Comment 18 Joacim Löwgren 2016-01-15 07:40:55 PST
Anything happening on this?
Comment 19 Michael Catanzaro 2016-09-17 07:18:11 PDT
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.
Comment 20 Mattia Astorino 2017-12-20 14:13:27 PST
Any update about this? Edge, Chrome and Firefox supports the <datalist> element..
Comment 21 Teun 2019-05-20 04:46:13 PDT
It appears that support for datalist has been added to Safari?
Tested in 12.1.1 and it works!
Comment 22 Michael Catanzaro 2019-05-20 06:47:50 PDT
Yes, but this is a GTK bug and it doesn't work for GTK yet.
Comment 23 Carlos Garcia Campos 2019-07-30 06:04:27 PDT
Created attachment 375155 [details]
Patch
Comment 24 Carlos Garcia Campos 2019-07-30 06:12:20 PDT
Created attachment 375156 [details]
Patch
Comment 25 Michael Catanzaro 2019-07-30 09:15:12 PDT
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?
Comment 26 Carlos Garcia Campos 2019-07-30 09:36:36 PDT
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.
Comment 27 Michael Catanzaro 2019-07-30 09:54:12 PDT
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.
Comment 28 Carlos Garcia Campos 2019-07-31 00:59:21 PDT
(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.
Comment 29 Carlos Garcia Campos 2019-07-31 01:02:16 PDT
Committed r248033: <https://trac.webkit.org/changeset/248033>