[webkit-reviews] review denied: [Bug 64690] [Qt][WK2] Implement load/back/forward/stop/reload for QDesktopWebView. : [Attachment 101153] Patch

[bugzilla-daemon at webkit.org]

Benjamin Poulain <benjamin at webkit.org> has denied Alexis Menard
<alexis.menard at openbossa.org>'s request for review:
Bug 64690: [Qt][WK2] Implement load/back/forward/stop/reload for
QDesktopWebView.
https://bugs.webkit.org/show_bug.cgi?id=64690

Attachment 101153: Patch
https://bugs.webkit.org/attachment.cgi?id=101153&action=review

------- Additional Comments from Benjamin Poulain <benjamin at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=101153&action=review


+ this needs tests :)

>> Source/WebKit2/ChangeLog:6
>> +	Just hook up to the QtWebPage proxy to trigger the appropriate actions.

> 
> Line contains tab character.	[whitespace/tab] [5]

Uh :)

> Source/WebKit2/UIProcess/API/qt/qdesktopwebview.cpp:202
> +void QDesktopWebView::back()
> +{
> +    d->page.triggerAction(QtWebPageProxy::Back);
> +}
> +
> +void QDesktopWebView::forward()
> +{
> +    d->page.triggerAction(QtWebPageProxy::Forward);
> +}
> +
> +void QDesktopWebView::reload()
> +{
> +    d->page.triggerAction(QtWebPageProxy::Reload);
> +}
> +
> +void QDesktopWebView::stop()
> +{
> +    d->page.triggerAction(QtWebPageProxy::Stop);
> +}
> +

Those should call navigationAction().

> Source/WebKit2/UIProcess/API/qt/qdesktopwebview.h:57
> +	void back();
> +	void forward();
> +	void reload();
> +	void stop();

I am not fan of this.
I like the "goBack", "goForward" from Cocoa's WebView :)

The stop, reload, back and forward action are all conditional to the state of
the engine. Because of that QAction make much more sense than those slots.