Bug 70525 - [Qt][WK2] Remove QAction from MiniBrowser
: [Qt][WK2] Remove QAction from MiniBrowser
Status: RESOLVED FIXED
: WebKit
WebKit Qt
: 528+ (Nightly build)
: Unspecified Unspecified
: P2 Normal
Assigned To:
:
:
:
: 70236 70315 70529
  Show dependency treegraph
 
Reported: 2011-10-20 10:59 PST by
Modified: 2011-10-27 06:20 PST (History)


Attachments
Patch for review. (29.43 KB, patch)
2011-10-20 12:07 PST, Zeno Albisser
no flags Review Patch | Details | Formatted Diff | Diff
patch for review. (33.40 KB, patch)
2011-10-24 02:51 PST, Zeno Albisser
no flags Review Patch | Details | Formatted Diff | Diff
patch for review. - fix failing tests after r98447. (1.99 KB, patch)
2011-10-26 16:27 PST, Zeno Albisser
no flags Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2011-10-20 10:59:06 PST
this is necessary to make the QWebNavigationController work nicely with QML.
------- Comment #1 From 2011-10-20 12:07:19 PST -------
Created an attachment (id=111819) [details]
Patch for review.
------- Comment #2 From 2011-10-24 02:25:27 PST -------
(From update of attachment 111819 [details])
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? :)
------- Comment #3 From 2011-10-24 02:51:56 PST -------
Created an attachment (id=112167) [details]
patch for review.
------- Comment #4 From 2011-10-24 02:54:40 PST -------
(From update of attachment 111819 [details])
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.
------- Comment #5 From 2011-10-25 05:29:24 PST -------
(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?
------- Comment #6 From 2011-10-25 05:44:05 PST -------
(In reply to comment #5)
> (From update of attachment 112167 [details] [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.
------- Comment #7 From 2011-10-25 08:45:08 PST -------
(From update of attachment 112167 [details])
This should be committed as part of a batch with all the other patches for https://bugs.webkit.org/show_bug.cgi?id=70315
------- Comment #8 From 2011-10-26 01:29:22 PST -------
Committed r98447: <http://trac.webkit.org/changeset/98447>
------- Comment #9 From 2011-10-26 06:31:19 PST -------
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)]
------- Comment #10 From 2011-10-26 16:27:46 PST -------
Created an attachment (id=112615) [details]
patch for review. - fix failing tests after r98447.
------- Comment #11 From 2011-10-27 06:20:03 PST -------
(From update of attachment 112615 [details])
Clearing flags on attachment: 112615

Committed r98560: <http://trac.webkit.org/changeset/98560>
------- Comment #12 From 2011-10-27 06:20:10 PST -------
All reviewed patches have been landed.  Closing bug.