| Summary: | [GTK] Initial loader client implementation in WebKitWebView | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Carlos Garcia Campos <cgarcia> | ||||||
| Component: | WebKit2 | Assignee: | Nobody <webkit-unassigned> | ||||||
| Status: | RESOLVED DUPLICATE | ||||||||
| Severity: | Normal | CC: | eric, mrobinson, xan.lopez | ||||||
| Priority: | P2 | ||||||||
| Version: | 528+ (Nightly build) | ||||||||
| Hardware: | PC | ||||||||
| OS: | Linux | ||||||||
| Bug Depends on: | 57820 | ||||||||
| Bug Blocks: | |||||||||
| Attachments: |
|
||||||||
|
Description
Carlos Garcia Campos
2011-04-05 05:47:45 PDT
Created attachment 88211 [details]
Patch
The patch applies on top of patch attached to bug #57820 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.
Created attachment 88441 [details] Updated patch Updated patch on top of patch attached to bug #57820. It also updates GtkLauncher instead of MiniBrowser 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. (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 on attachment 88441 [details]
Updated patch
Going to remove the r? flag here until we can resolve API discussions.
|