Bug 57835 - [GTK] Initial loader client implementation in WebKitWebView
Summary: [GTK] Initial loader client implementation in WebKitWebView
Status: RESOLVED DUPLICATE of bug 68085
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on: 57820
Blocks:
  Show dependency treegraph
 
Reported: 2011-04-05 05:47 PDT by Carlos Garcia Campos
Modified: 2011-09-26 03:50 PDT (History)
3 users (show)

See Also:


Attachments
Patch (15.83 KB, patch)
2011-04-05 05:54 PDT, Carlos Garcia Campos
no flags Details | Formatted Diff | Diff
Updated patch (15.39 KB, patch)
2011-04-06 09:08 PDT, Carlos Garcia Campos
no flags 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 2011-04-05 05:47:45 PDT
I would allow the minibrowser to update the window title and location entry.
Comment 1 Carlos Garcia Campos 2011-04-05 05:54:08 PDT
Created attachment 88211 [details]
Patch
Comment 2 Carlos Garcia Campos 2011-04-05 05:54:54 PDT
The patch applies on top of patch attached to bug #57820
Comment 3 Martin Robinson 2011-04-05 08:25:30 PDT
Comment on attachment 88211 [details]
Patch

We should allow applications to use either the WebKit1 or WebKit2 style API. Installing a Client on the widget seems to add a lot of extra overhead. Perhaps we can make them separate widgets.
Comment 4 Carlos Garcia Campos 2011-04-06 09:08:21 PDT
Created attachment 88441 [details]
Updated patch

Updated patch on top of patch attached to bug #57820. It also updates GtkLauncher instead of MiniBrowser
Comment 5 Alejandro G. Castro 2011-04-08 13:07:33 PDT
Comment on attachment 88441 [details]
Updated patch

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

> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:120
> +
> +    g_object_class_install_property(gobjectClass,
> +                                    PROP_TITLE,
> +                                    g_param_spec_string("title",
> +                                                        "Title",
> +                                                        "Returns the @web_view's document title",
> +                                                        0,
> +                                                        G_PARAM_READABLE));
> +    g_object_class_install_property(gobjectClass,
> +                                    PROP_URI,
> +                                    g_param_spec_string("uri",
> +                                                        "URI",
> +                                                        "Returns the current URI of the contents displayed by the @web_view",
> +                                                        0,
> +                                                        G_PARAM_READABLE));
> +    g_object_class_install_property(gobjectClass,
> +                                    PROP_PROGRESS,
> +                                    g_param_spec_double("progress",
> +                                                        "Progress",
> +                                                        "Determines the current progress of the load",
> +                                                        0.0, 1.0, 1.0,
> +                                                        G_PARAM_READABLE));

Maybe stating to add the doc of the API it is a good idea.

> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:139
> +    // TODO: update load-status property

In this cases we are using NotImplemented without the TODO comment, I would stick to that and add the comment just in case it adds any other information.

> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:287
> +    WKPageLoaderClient loadClient = {
> +        0,       /* version */
> +        webView, /* clientInfo */
> +        didStartProvisionalLoadForFrame,
> +        didReceiveServerRedirectForProvisionalLoadForFrame,
> +        didFailProvisionalLoadWithErrorForFrame,
> +        didCommitLoadForFrame,
> +        didFinishDocumentLoadForFrame,
> +        didFinishLoadForFrame,
> +        didFailLoadWithErrorForFrame,
> +        0, /* didSameDocumentNavigationForFrame */
> +        didReceiveTitleForFrame,
> +        didFirstLayoutForFrame,
> +        didFirstVisuallyNonEmptyLayoutForFrame,
> +        didRemoveFrameFromHierarchy,
> +        0, /* didDisplayInsecureContentForFrame */
> +        0, /* didRunInsecureContentForFrame */
> +        0, /* canAuthenticateAgainstProtectionSpaceInFrame */
> +        0, /* didReceiveAuthenticationChallengeInFrame */
> +        didStartProgress,
> +        didChangeProgress,
> +        didFinishProgress,
> +        didBecomeUnresponsive,
> +        didBecomeResponsive,
> +        0,  /* processDidCrash */
> +        0,  /* didChangeBackForwardList */
> +        0   /* shouldGoToBackForwardListItem */
> +    };

Shall we add all these functions to a different file? Maybe PageLoaderClient.
Comment 6 Carlos Garcia Campos 2011-04-10 23:37:27 PDT
(In reply to comment #5)
> (From update of attachment 88441 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=88441&action=review
> 
> > Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:120
> > +
> > +    g_object_class_install_property(gobjectClass,
> > +                                    PROP_TITLE,
> > +                                    g_param_spec_string("title",
> > +                                                        "Title",
> > +                                                        "Returns the @web_view's document title",
> > +                                                        0,
> > +                                                        G_PARAM_READABLE));
> > +    g_object_class_install_property(gobjectClass,
> > +                                    PROP_URI,
> > +                                    g_param_spec_string("uri",
> > +                                                        "URI",
> > +                                                        "Returns the current URI of the contents displayed by the @web_view",
> > +                                                        0,
> > +                                                        G_PARAM_READABLE));
> > +    g_object_class_install_property(gobjectClass,
> > +                                    PROP_PROGRESS,
> > +                                    g_param_spec_double("progress",
> > +                                                        "Progress",
> > +                                                        "Determines the current progress of the load",
> > +                                                        0.0, 1.0, 1.0,
> > +                                                        G_PARAM_READABLE));
> 
> Maybe stating to add the doc of the API it is a good idea.

Yes, it's a matter of copy and paste from webkit1, I haven't included the docs in this first patches to make them simple. 

> > Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:139
> > +    // TODO: update load-status property
> 
> In this cases we are using NotImplemented without the TODO comment, I would stick to that and add the comment just in case it adds any other information.

Ok, I'll add it, the TDO comment was to remember that load-status property is what should be updated there, and not emit the load-* signals that are deprecated. Hopefully this won't be unimplemented for long time :-)

> > Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:287
> > +    WKPageLoaderClient loadClient = {
> > +        0,       /* version */
> > +        webView, /* clientInfo */
> > +        didStartProvisionalLoadForFrame,
> > +        didReceiveServerRedirectForProvisionalLoadForFrame,
> > +        didFailProvisionalLoadWithErrorForFrame,
> > +        didCommitLoadForFrame,
> > +        didFinishDocumentLoadForFrame,
> > +        didFinishLoadForFrame,
> > +        didFailLoadWithErrorForFrame,
> > +        0, /* didSameDocumentNavigationForFrame */
> > +        didReceiveTitleForFrame,
> > +        didFirstLayoutForFrame,
> > +        didFirstVisuallyNonEmptyLayoutForFrame,
> > +        didRemoveFrameFromHierarchy,
> > +        0, /* didDisplayInsecureContentForFrame */
> > +        0, /* didRunInsecureContentForFrame */
> > +        0, /* canAuthenticateAgainstProtectionSpaceInFrame */
> > +        0, /* didReceiveAuthenticationChallengeInFrame */
> > +        didStartProgress,
> > +        didChangeProgress,
> > +        didFinishProgress,
> > +        didBecomeUnresponsive,
> > +        didBecomeResponsive,
> > +        0,  /* processDidCrash */
> > +        0,  /* didChangeBackForwardList */
> > +        0   /* shouldGoToBackForwardListItem */
> > +    };
> 
> Shall we add all these functions to a different file? Maybe PageLoaderClient.

Yes, there's a FIXME with the same question indeed.

Thanks
Comment 7 Martin Robinson 2011-04-26 16:58:48 PDT
Comment on attachment 88441 [details]
Updated patch

Going to remove the r? flag here until we can resolve API discussions.
Comment 8 Carlos Garcia Campos 2011-09-26 03:50:38 PDT
I had forgotten this, marking as dupl.

*** This bug has been marked as a duplicate of bug 68085 ***