Bug 241420 - [GTK][WPE] Add new JavaScript execution APIs
Summary: [GTK][WPE] Add new JavaScript execution APIs
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Carlos Garcia Campos
URL:
Keywords:
Depends on:
Blocks: GTK4
  Show dependency treegraph
 
Reported: 2022-06-08 09:24 PDT by Michael Catanzaro
Modified: 2023-02-03 23:04 PST (History)
5 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Catanzaro 2022-06-08 09:24:12 PDT
In the new WPE/GTK API, we no longer need both webkit_web_view_run_javascript() and _run_javascript_in_world() functions. We can keep only _run_javascript(), and give it the world parameter.
Comment 1 Michael Catanzaro 2022-06-09 05:47:34 PDT
Should also split the script argument into:

  @body: the JavaScript function body
  @arguments: a #GHashTable storing the function arguments

to match webkit_web_view_run_async_javascript_function_in_world() added in https://github.com/WebKit/WebKit/pull/1378.
Comment 2 Daniel Kolesa 2022-07-26 12:42:09 PDT
There is a caveat that I came across when debugging https://github.com/WebKit/WebKit/pull/1378. The PR seemingly assumes that empty string world name == NULL world name == default world. This is not the case, as the default world is completely isolated.

However, I believe we could reserve NULL input in the world parameter as a special case for the default world, and treat any non-NULL argument as a specific world name. That would enable the existing APIs to work (it effectively only loosens their behavior) while allowing to drop the others.

The API introduced in the PR also needs to be fixed. I will create a pull request to fix it.
Comment 3 Carlos Garcia Campos 2023-02-01 02:38:02 PST
(In reply to Michael Catanzaro from comment #1)
> Should also split the script argument into:
> 
>   @body: the JavaScript function body
>   @arguments: a #GHashTable storing the function arguments
> 
> to match webkit_web_view_run_async_javascript_function_in_world() added in
> https://github.com/WebKit/WebKit/pull/1378.

That only makes sense when calling a function. I'm not sure we want a function for everything with optional parameters, so at least I would keep two, once for evaluating a script and another for calling a function. Maybe we can indeed use "evaluate" and "call" instead of "run" (matching cocoa api) and deprecate the run methods, which would simplify the ifdefs logic and api documentation. The equivalent to what we currently have would be:

void webkit_web_view_evaluate_javascript(WebKitWebView* webView, const char* script, const char* worldName, GCancellable* cancellable, GAsyncReadyCallback callback, gpointer userData)

void webkit_web_view_call_async_javascript_function(WebKitWebView* webView, const char* body, GVariant* arguments, const char* worldName, GCancellable* cancellable, GAsyncReadyCallback callback, gpointer userData)

I don't know what to do with webkit_web_view_run_javascript_from_gresource(). It doesn't have the in_world variant, nor the async functiona call. I think we can just remove it from the new API, callers can read the resource first and then use evaluate or call.
Comment 4 Michael Catanzaro 2023-02-01 06:47:05 PST
(In reply to Carlos Garcia Campos from comment #3)
> void webkit_web_view_evaluate_javascript(WebKitWebView* webView, const char*
> script, const char* worldName, GCancellable* cancellable,
> GAsyncReadyCallback callback, gpointer userData)
> 
> void webkit_web_view_call_async_javascript_function(WebKitWebView* webView,
> const char* body, GVariant* arguments, const char* worldName, GCancellable*
> cancellable, GAsyncReadyCallback callback, gpointer userData)

Sounds fine to me. Please get more opinions though (at least from Adrian and Phil).

It's _slightly_ confusing that only the second function name contains the word "async" when they're both async, but I see why it's done that way: the webkit_web_view_call_async_javascript_function() only works for a JS function that returns a Promise.

> I don't know what to do with
> webkit_web_view_run_javascript_from_gresource(). It doesn't have the
> in_world variant, nor the async functiona call. I think we can just remove
> it from the new API, callers can read the resource first and then use
> evaluate or call.

It's convenient, but OK.
Comment 5 Carlos Garcia Campos 2023-02-02 01:20:21 PST
(In reply to Michael Catanzaro from comment #4)
> (In reply to Carlos Garcia Campos from comment #3)
> > void webkit_web_view_evaluate_javascript(WebKitWebView* webView, const char*
> > script, const char* worldName, GCancellable* cancellable,
> > GAsyncReadyCallback callback, gpointer userData)
> > 
> > void webkit_web_view_call_async_javascript_function(WebKitWebView* webView,
> > const char* body, GVariant* arguments, const char* worldName, GCancellable*
> > cancellable, GAsyncReadyCallback callback, gpointer userData)
> 
> Sounds fine to me. Please get more opinions though (at least from Adrian and
> Phil).
> 
> It's _slightly_ confusing that only the second function name contains the
> word "async" when they're both async, but I see why it's done that way: the
> webkit_web_view_call_async_javascript_function() only works for a JS
> function that returns a Promise.

async in this case doesn't refer to the API call, but to the js function, see https://tc39.es/ecma262/multipage/ecmascript-language-functions-and-classes.html#sec-async-function-definitions
Comment 6 Carlos Garcia Campos 2023-02-02 06:18:48 PST
Pull request: https://github.com/WebKit/WebKit/pull/9515
Comment 7 EWS 2023-02-03 23:04:10 PST
Committed 259852@main (c07c7ce77b19): <https://commits.webkit.org/259852@main>

Reviewed commits have been landed. Closing PR #9515 and removing active labels.