Code review comment for lp:~uriboni/webbrowser-app/find-in-page

Revision history for this message
Olivier Tilloy (osomon) wrote :

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.

review: Needs Fixing

« Back to merge proposal