Bug 70525 - [Qt][WK2] Remove QAction from MiniBrowser
Summary: [Qt][WK2] Remove QAction from MiniBrowser
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Qt (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Zeno Albisser
URL:
Keywords:
Depends on:
Blocks: 70236 70315 70529
  Show dependency treegraph
 
Reported: 2011-10-20 10:59 PDT by Zeno Albisser
Modified: 2011-10-27 06:20 PDT (History)
3 users (show)

See Also:


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

Note You need to log in before you can comment on or make changes to this bug.
Description Zeno Albisser 2011-10-20 10:59:06 PDT
this is necessary to make the QWebNavigationController work nicely with QML.
Comment 1 Zeno Albisser 2011-10-20 12:07:19 PDT
Created attachment 111819 [details]
Patch for review.
Comment 2 Simon Hausmann 2011-10-24 02:25:27 PDT
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? :)
Comment 3 Zeno Albisser 2011-10-24 02:51:56 PDT
Created attachment 112167 [details]
patch for review.
Comment 4 Zeno Albisser 2011-10-24 02:54:40 PDT
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.
Comment 5 Simon Hausmann 2011-10-25 05:29:24 PDT
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?
Comment 6 Zeno Albisser 2011-10-25 05:44:05 PDT
(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.
Comment 7 Zeno Albisser 2011-10-25 08:45:08 PDT
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
Comment 8 Simon Hausmann 2011-10-26 01:29:22 PDT
Committed r98447: <http://trac.webkit.org/changeset/98447>
Comment 9 Csaba Osztrogonác 2011-10-26 06:31:19 PDT
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 Zeno Albisser 2011-10-26 16:27:46 PDT
Created attachment 112615 [details]
patch for review. - fix failing tests after r98447.
Comment 11 Andras Becsi 2011-10-27 06:20:03 PDT
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>
Comment 12 Andras Becsi 2011-10-27 06:20:10 PDT
All reviewed patches have been landed.  Closing bug.