[Webkit-unassigned] [Bug 133680] [GTK] WebKitWebView::create should receive information about the navigation action

[bugzilla-daemon at webkit.org]

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





--- Comment #4 from Martin Robinson <mrobinson at webkit.org>  2014-06-23 09:32:59 PST ---
(From update of attachment 232791)
View in context: https://bugs.webkit.org/attachment.cgi?id=232791&action=review

Seems okay, though I have a few  nits. I think, if possible, it'd be better to pass WebHitTestResult& everywhere instead of the internal data structure. Probably a good idea to fix up the style errors too. :)

> Source/WebKit2/UIProcess/API/gtk/WebKitNavigationActionPrivate.h:49
> +    unsigned isUserGesture : 1;

Why not use a bool here?

> Source/WebKit2/UIProcess/API/gtk/WebKitUIClient.cpp:50
> +        WebKitNavigationAction navigation(request.get(), navigationActionData);
> +        return webkitWebViewCreateNewPage(m_webView, windowFeatures, &navigation);

Nit: Probably "action" is a better name.

> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:940
> +        g_cclosure_marshal_generic,

Is this really the correct marshaller for a OBJECT and an enum value?

> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:1858
> +    // FIXME: we should use a custom ContextMenuclient at some point.

Nit: ContextMenuclient -> ContextMenuClient.

> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:1869
> +    WebHitTestResult::Data data;
> +    data.absoluteImageURL = webHitTestResult->absoluteImageURL();
> +    data.absoluteLinkURL = webHitTestResult->absoluteLinkURL();
> +    data.absoluteMediaURL = webHitTestResult->absoluteMediaURL();
> +    data.linkLabel = webHitTestResult->linkLabel();
> +    data.linkTitle = webHitTestResult->linkTitle();
> +    data.isContentEditable = webHitTestResult->isContentEditable();
> +    data.elementBoundingBox = webHitTestResult->elementBoundingBox();
> +    data.isScrollbar = webHitTestResult->isScrollbar();
> +
> +    GRefPtr<WebKitHitTestResult> hitTestResult = adoptGRef(webkitHitTestResultCreate(data));

Wow. It'd be great to avoid all these copies! Why not just pass a const WebHitTestResult& instead of Data?

> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.h:212
> +                                                WebKitNavigationAction      *navigation);

Probably should be called action or navigation_action.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.