Bug 70525 – [Qt][WK2] Remove QAction from MiniBrowser

• Component: WebKit Qt
• Status: RESOLVED FIXED

[Zeno Albisser]

this is necessary to make the QWebNavigationController work nicely with QML.

[Zeno Albisser]

Created attachment 111819 [details]
Patch for review.

[Simon Hausmann]

Comment on attachment 111819 [details]
Patch for review.

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

> Source/WebKit2/UIProcess/API/qt/qwebnavigationcontroller.h:55
> +    Q_INVOKABLE void goBack();
> +    Q_INVOKABLE void goForward();
> +    Q_INVOKABLE void stop();
> +    Q_INVOKABLE void reload();

Why are those functions marked as Q_INVOKABLE when they're already in a slot section? :)

[Zeno Albisser]

Created attachment 112167 [details]
patch for review.

[Zeno Albisser]

Comment on attachment 111819 [details]
Patch for review.

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

>> Source/WebKit2/UIProcess/API/qt/qwebnavigationcontroller.h:55
>> +    Q_INVOKABLE void reload();
> 
> Why are those functions marked as Q_INVOKABLE when they're already in a slot section? :)

fixed.

[Simon Hausmann]

Comment on attachment 112167 [details]
patch for review.

r=me

Perhaps this patch should be landed together with the other MiniBrowser ones in one shot, to avoid having a broken mini browser inbetween. (Well, one can argue the current one is too broken ;)

What do you prefer?

[Zeno Albisser]

(In reply to comment #5)
> (From update of attachment 112167 [details])
> r=me
> 
> Perhaps this patch should be landed together with the other MiniBrowser ones in one shot, to avoid having a broken mini browser inbetween. (Well, one can argue the current one is too broken ;)
> 
> What do you prefer?

We wanted to have that one separate because of the changes in the API (QWebNavigationController). I think it makes sense to keep it separate. But don't have a really strong preference either.

[Zeno Albisser]

Comment on attachment 112167 [details]
patch for review.

This should be committed as part of a batch with all the other patches for https://bugs.webkit.org/show_bug.cgi?id=70315

[Simon Hausmann]

Committed r98447: <http://trac.webkit.org/changeset/98447>

[Csaba Osztrogonác]

It broke 2 API tests, reopen to fix them:

FAIL!  : tst_CommonViewTests::backAndForward() 'waitForSignal(viewAbstraction.data(), SIGNAL(loadSucceeded()))' returned FALSE. ()
   Loc: [/ramdisk/qt-linux-32-release-webkit2/build/Source/WebKit2/UIProcess/API/qt/tests/commonviewtests/tst_commonviewtests.cpp(121)]

   FAIL!  : tst_CommonViewTests::reload() 'waitForSignal(viewAbstraction.data(), SIGNAL(loadSucceeded()))' returned FALSE. ()
   Loc: [/ramdisk/qt-linux-32-release-webkit2/build/Source/WebKit2/UIProcess/API/qt/tests/commonviewtests/tst_commonviewtests.cpp(143)]

[Zeno Albisser]

Created attachment 112615 [details]
patch for review. - fix failing tests after r98447.

[Andras Becsi]

Comment on attachment 112615 [details]
patch for review. - fix failing tests after r98447.

Clearing flags on attachment: 112615

Committed r98560: <http://trac.webkit.org/changeset/98560>

[Andras Becsi]

All reviewed patches have been landed.  Closing bug.