Bug 175259 - [GTK] Implement JavaScript dialog methods of API::AutomationSessionClient
Summary: [GTK] Implement JavaScript dialog methods of API::AutomationSessionClient
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: Gtk
Depends on:
Blocks: 175261
  Show dependency treegraph
 
Reported: 2017-08-07 03:15 PDT by Carlos Garcia Campos
Modified: 2017-08-07 23:25 PDT (History)
3 users (show)

See Also:


Attachments
Patch (28.45 KB, patch)
2017-08-07 03:29 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 Carlos Garcia Campos 2017-08-07 03:15:38 PDT
Needed for the web driver prompt commands. See bug #174614
Comment 1 Carlos Garcia Campos 2017-08-07 03:29:16 PDT
Created attachment 317408 [details]
Patch
Comment 2 BJ Burg 2017-08-07 10:06:56 PDT
Comment on attachment 317408 [details]
Patch

WebDriver related parts look good to me. I'd like Michael or another GTK reviewer to look too.
Comment 3 Michael Catanzaro 2017-08-07 12:05:16 PDT
Comment on attachment 317408 [details]
Patch

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

> Source/WebCore/platform/gtk/po/POTFILES.in:33
> +../../../WebKit/UIProcess/API/gtk/WebKitScriptDialogGtk.cpp

Do we have a script that reminds you to do this, or are you the hero of the translators?

> Source/WebKit/ChangeLog:11
> +        files. Implement all JavaScript dialog methods of API::AutomationSessionClient in WebKitAutomationSession. For
> +        now it only works when the user doesn't override WebKitWebView::script-dialog signal and default implementation
> +        is used.

Well that's a significant limitation.

> Source/WebKit/UIProcess/API/glib/WebKitWebView.cpp:2051
> +    // FIXME: Add API to ask the user in case default implementation is not being used.

What should that API look like?
Comment 4 Carlos Garcia Campos 2017-08-07 22:42:25 PDT
(In reply to Michael Catanzaro from comment #3)
> Comment on attachment 317408 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=317408&action=review
> 
> > Source/WebCore/platform/gtk/po/POTFILES.in:33
> > +../../../WebKit/UIProcess/API/gtk/WebKitScriptDialogGtk.cpp
> 
> Do we have a script that reminds you to do this, or are you the hero of the
> translators?

Not that I know of, I guess I'm the hero of the translators, then :-D

> > Source/WebKit/ChangeLog:11
> > +        files. Implement all JavaScript dialog methods of API::AutomationSessionClient in WebKitAutomationSession. For
> > +        now it only works when the user doesn't override WebKitWebView::script-dialog signal and default implementation
> > +        is used.
> 
> Well that's a significant limitation.

It would be significant if applications actually overrode the default implementation, but I bet nobody is doing that (I don't really know, though).

> > Source/WebKit/UIProcess/API/glib/WebKitWebView.cpp:2051
> > +    // FIXME: Add API to ask the user in case default implementation is not being used.
> 
> What should that API look like?

I don't know, but more or less the same we are currently using internally, but using signals to ask the user about the js dialogs. Or we could add a WebKitScriptDialogManager or something like that, to not add more signals to the WebKitWebView. I don't know i haven't thought about it yet.
Comment 5 Carlos Garcia Campos 2017-08-07 23:25:34 PDT
Committed r220387: <http://trac.webkit.org/changeset/220387>