[Merge] lp:~uriboni/webbrowser-app/find-in-page into lp:webbrowser-app

Olivier Tilloy olivier.tilloy at canonical.com
Thu May 28 19:11:43 UTC 2015


Review: Needs Fixing

When running the unit tests in verbose mode, I’m seeing the following warning:

    QWARN  : QmlTests::AddressBar::test_exitingFindInPageRestoresUrl() file://…/src/app/webbrowser/AddressBar.qml:58:15: Unable to assign [undefined] to bool

This could easily be addressed by setting findController to a mock in the unit test.


Since UbuntuWebView02.qml imports oxide 1.8, the runtime dependency of qtdeclarative5-ubuntu-web-plugin on liboxideqt-qmlplugin needs to be bumped to 1.8 in debian/control.


In AddressBar.qml:
 - I don’t think the changes to the 'visible' property of the favicon and the 'name' property of the Icon with id 'action' are useful, given that the parent item is invisible when findInPageMode is true.
 - In the findInPageCounter Label, instead of #5d5d5d, use UbuntuColors.darkGrey for the color.


In Browser.qml:
 - The 'findinpage' action should be enabled only if !chrome.findInPageMode.

-- 
https://code.launchpad.net/~uriboni/webbrowser-app/find-in-page/+merge/258225
Your team Ubuntu Phablet Team is subscribed to branch lp:webbrowser-app.



More information about the Ubuntu-reviews mailing list