[Webkit-unassigned] [Bug 157899] [GTK] Provide frame-related load signals in WebKitWebView

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=157899

Carlos Garcia Campos <cgarcia at igalia.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #279764|review?, commit-queue?      |review-, commit-queue-
              Flags|                            |

--- Comment #14 from Carlos Garcia Campos <cgarcia at igalia.com> ---
Comment on attachment 279764
  --> https://bugs.webkit.org/attachment.cgi?id=279764
proposed patch IV

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

Sorry, I forgot about this new patch. I don't think this is what we want though, see my comments below

> Source/WebKit2/WebProcess/InjectedBundle/API/gtk/WebKitWebPage.cpp:60
> +    FRAME_LOADED,

I don't think this belongs here. This could makes sense in the UI process where we don't expose frames, but in the web extensions API we have WebKitFrame, so there's no reason for adding frame related signals to the page object. One of the reasons why we didn't want to expose frames was because we had a lot of signals duplicated in web view and frame, making it confusing. The web view signals actually meant main frame events, so we had two ways of getting events for the main frame. I agree we would be making the same mistake again here if we add document-loaded signal to WebKitFrame. Fortunately in the web extensions API we only have one signal related to the frame, document-loaded, so maybe we can deprecate it and add a new one that receives the frame. Or deprecated it, and move it to WebKitFrame (I know we only have API to get the main frame yet, but we could add frame-created signal or something like that).

> Source/WebKit2/WebProcess/InjectedBundle/API/gtk/WebKitWebPage.cpp:186
> +        g_signal_emit(WEBKIT_WEB_PAGE(clientInfo), signals[FRAME_LOADED], 0, toImpl(frame)->coreFrame()->document()->url().string().utf8().data());

This is not frame-loaded, this should be frame-document-loaded or document-loaded-in-frame and it should receive a WebKitFrame, not a document URL.

> Tools/TestWebKitAPI/Tests/WebKit2Gtk/TestWebExtensions.cpp:81
> +    const char* frame_uri = nullptr;

frame_uri -> frameURI

> Tools/TestWebKitAPI/Tests/WebKit2Gtk/WebExtensionTest.cpp:139
> +static void frameLoadedCallback(WebKitWebPage* webPage, const gchar* frame_uri, WebKitWebExtension* extension)

Ditto.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20160601/3e2a926a/attachment-0001.html>