Bug 98934

Summary:

[GTK] Datalist element support for TextFieldInputType

Product:

WebKit

Reporter:

Zan Dobersek <zan>

Component:

WebKitGTK

Assignee:

ChangSeok Oh <changseok>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

andersca, a.renevier, astorino.design, bugs-noreply, cgarcia, changseok, chi187, commit-queue, eflews.bot, esprehn+autocc, glenn, gustavo, gyuyoung.kim, joacim.lowgren, joone, kling, macpherson, mcatanzaro, menard, mike, mrobinson, rakuco, teun

Priority:

Keywords:

Gtk, LayoutTestFailure

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

122590, 122819

Bug Blocks:

27247

Attachments:

Description	Flags
Patch	none
Patch	eflews.bot: commit-queue-
Patch	none
Patch	none
Add a test to verify the functionality	none
Patch	none
Patch	none
Patch	mcatanzaro: review+

Zan Dobersek

Reported 2012-10-10 11:44:16 PDT

Enable the datalist element support and unskip related tests.

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
Show Obsolete (7) View All Add attachment proposed patch, testcase, etc.

ChangSeok Oh

Comment 1 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.

Zan Dobersek

Comment 2 2013-10-03 09:45:55 PDT

Nope, feel free to hack on this.

ChangSeok Oh

Comment 3 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.

ChangSeok Oh

Comment 4 2013-12-10 14:56:53 PST

Created attachment 218910 [details] Patch

WebKit Commit Bot

Comment 5 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.

EFL EWS Bot

Comment 6 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

EFL EWS Bot

Comment 7 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

ChangSeok Oh

Comment 8 2013-12-11 03:19:16 PST

Created attachment 218947 [details] Patch

WebKit Commit Bot

Comment 9 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.

ChangSeok Oh

Comment 10 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

EFL EWS Bot

Comment 11 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

ChangSeok Oh

Comment 12 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.. :(

ChangSeok Oh

Comment 13 2014-01-17 01:32:00 PST

Comment on attachment 218947 [details] Patch I got a better idea using shadow html like media controls.

ChangSeok Oh

Comment 14 2014-02-06 10:55:07 PST

Created attachment 223347 [details] Patch

ChangSeok Oh

Comment 15 2014-02-06 11:12:56 PST

Created attachment 223351 [details] Patch

ChangSeok Oh

Comment 16 2014-02-07 00:26:12 PST

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

ChangSeok Oh

Comment 17 2014-02-07 01:47:14 PST

Created attachment 223433 [details] Patch

Joacim Löwgren

Comment 18 2016-01-15 07:40:55 PST

Anything happening on this?

Michael Catanzaro

Comment 19 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.

Mattia Astorino

Comment 20 2017-12-20 14:13:27 PST

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

Teun

Comment 21 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!

Michael Catanzaro

Comment 22 2019-05-20 06:47:50 PDT

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

Carlos Garcia Campos

Comment 23 2019-07-30 06:04:27 PDT

Created attachment 375155 [details] Patch

Carlos Garcia Campos

Comment 24 2019-07-30 06:12:20 PDT

Created attachment 375156 [details] Patch

Michael Catanzaro

Comment 25 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?

Carlos Garcia Campos

Comment 26 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.

Michael Catanzaro

Comment 27 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.

Carlos Garcia Campos

Comment 28 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.

Carlos Garcia Campos

Comment 29 2019-07-31 01:02:16 PDT

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

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