RESOLVED FIXED 70525
[Qt][WK2] Remove QAction from MiniBrowser
https://bugs.webkit.org/show_bug.cgi?id=70525
Summary [Qt][WK2] Remove QAction from MiniBrowser
Zeno Albisser
Reported 2011-10-20 10:59:06 PDT
this is necessary to make the QWebNavigationController work nicely with QML.
Attachments
Patch for review. (29.43 KB, patch)
2011-10-20 12:07 PDT, Zeno Albisser
no flags
patch for review. (33.40 KB, patch)
2011-10-24 02:51 PDT, Zeno Albisser
no flags
patch for review. - fix failing tests after r98447. (1.99 KB, patch)
2011-10-26 16:27 PDT, Zeno Albisser
no flags
Zeno Albisser
Comment 1 2011-10-20 12:07:19 PDT
Created attachment 111819 [details] Patch for review.
Simon Hausmann
Comment 2 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? :)
Zeno Albisser
Comment 3 2011-10-24 02:51:56 PDT
Created attachment 112167 [details] patch for review.
Zeno Albisser
Comment 4 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.
Simon Hausmann
Comment 5 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?
Zeno Albisser
Comment 6 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.
Zeno Albisser
Comment 7 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
Simon Hausmann
Comment 8 2011-10-26 01:29:22 PDT
Csaba Osztrogonác
Comment 9 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)]
Zeno Albisser
Comment 10 2011-10-26 16:27:46 PDT
Created attachment 112615 [details] patch for review. - fix failing tests after r98447.
Andras Becsi
Comment 11 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>
Andras Becsi
Comment 12 2011-10-27 06:20:10 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.