webbrowser-app

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

  1. find-in-page
  2. Merge into trunk
Proposed by Ugo Riboni
Status: Rejected
Rejected by: Olivier Tilloy
Proposed branch: lp:~uriboni/webbrowser-app/find-in-page
Merge into: lp:webbrowser-app
Diff against target: 647 lines (+351/-51)
9 files modified
debian/control (+2/-2)
src/app/actions/FindInPage.qml (+27/-0)
src/app/webbrowser/AddressBar.qml (+63/-22)
src/app/webbrowser/Browser.qml (+23/-6)
src/app/webbrowser/Chrome.qml (+71/-21)
tests/autopilot/webbrowser_app/emulators/browser.py (+11/-0)
tests/autopilot/webbrowser_app/tests/http_server.py (+5/-0)
tests/autopilot/webbrowser_app/tests/test_findinpage.py (+134/-0)
tests/unittests/qml/tst_AddressBar.qml (+15/-0)
To merge this branch: bzr merge lp:~uriboni/webbrowser-app/find-in-page
Reviewer Review Type Date Requested Status
Olivier Tilloy Disapprove
Riccardo Padovani (community) Needs Fixing
PS Jenkins bot continuous-integration Needs Fixing
Ugo Riboni (community) Approve
Review via email: mp+258225@code.launchpad.net

Commit message

Implement the "Find in Page" feature.
This bumps the runtime dependency on liboxideqt-qmlplugin to 1.8.

Description of the change

Implement the Find in Page feature in the browser UI according to the UX spec.

The only difference from the spec is the fact that results counter comes before the clear button. This is because the UI Toolkit component TextField does not support adding components after the clear button.

Needs this oxide branch to work: https://code.launchpad.net/~uriboni/oxide/find-in-page/+merge/258184

To post a comment you must log in.
Revision history for this message
Olivier Tilloy (osomon) wrote :

Haven’t done a proper review yet, but here’s one preliminary remark: you’ll need to bump the dependency on liboxideqt-qmlplugin to >= 1.8 in debian/control, for webbrowser-app and qtdeclarative5-ubuntu-web-plugin.

review: Needs Fixing
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:986
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/1723/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/2626/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-amd64-ci/480/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/480/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-i386-ci/480/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/2624/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/webbrowser-app-ci/1723/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:991
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/1725/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/2637/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-amd64-ci/482/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/482/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-i386-ci/482/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/2635/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/webbrowser-app-ci/1725/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Olivier Tilloy (osomon) wrote :

Thanks for working on this. I did a first quick round of testing review, here are a few comments, in no particular order:

The flake8 unit test is failing.

The label of findInPageCounter should be i18n’d, with a proper TRANSLATORS comment.
As a consequence, autopilot tests shouldn’t rely on the value of that label. Can we somehow expose the values of current and count as properties of the label, for testing purposes?

It appears findInPageMode is never assigned a default value. Not sure whether javascript (and its QML implementation) specifies that booleans are false by default, but it would be cautious to have it default to false.

87 + value: textField.text.length > 1 ? textField.text : ""
  Shouldn’t the above be [textField.text.length > 0] ? Or is it intentional that searching doesn’t start for one single character? I can’t open the UX spec right now, if this is intentional would you mind adding a comment to explain the rationale?

« This is because the UI Toolkit component TextField does not support adding components after the clear button »
  Can you raise that with design (James Mulholland and the UITK team, to ensure they are aware of the issue?

The size of ChromeButton instances in Chrome.qml has been updated at revision 997, can you please update the new instances too?

In the autopilot emulator for the chrome, the new buttons could use a more explicit naming scheme, e.g. "find_next_button()" and "find_prev_button()".

review: Needs Fixing
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:992
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/1747/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/2747/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-amd64-ci/504/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/504/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-i386-ci/504/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/2745/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/webbrowser-app-ci/1747/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Ugo Riboni (uriboni) wrote :

Fixed everything except the following:

> The label of findInPageCounter should be i18n’d, with a proper TRANSLATORS
> comment.
> As a consequence, autopilot tests shouldn’t rely on the value of that label.
> Can we somehow expose the values of current and count as properties of the
> label, for testing purposes?

I disagree with this. In my opinion numbers are not language specific and the "/" is just a random separator.
Who is going to translate a slash ?

> It appears findInPageMode is never assigned a default value. Not sure whether
> javascript (and its QML implementation) specifies that booleans are false by
> default, but it would be cautious to have it default to false.

Variables are not initialized in JS and an uninitialized variable when interpreted in boolean context has a false value, but I have fixed this for clarity.

> « This is because the UI Toolkit component TextField does not support adding
> components after the clear button »
> Can you raise that with design (James Mulholland and the UITK team, to
> ensure they are aware of the issue?

I spoke with Rae who spoke with James who said for now it is ok to do it the way we do it, and they will see with the UI Toolkit people if this can be changed in the future.

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

Thanks for the changes, they look good.

> I disagree with this. In my opinion numbers are not language specific
> and the "/" is just a random separator.
> Who is going to translate a slash ?

That’s an opinion, not an informed decision. The "n/m" formatting might look awkward is some languages, or maybe even completely meaningless. Can you check that this formatting is ok for all languages listed at https://en.wikipedia.org/wiki/List_of_language_names ? If you can’t, then the string should be i18n’d.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:998
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/1753/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/2771/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-amd64-ci/510/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/510/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-i386-ci/510/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/2769/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/webbrowser-app-ci/1753/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Ugo Riboni (uriboni) wrote :

Looks good and fixes the issue

review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:999
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/1754/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/2774/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-amd64-ci/511/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/511/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-i386-ci/511/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/2772/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/webbrowser-app-ci/1754/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Ugo Riboni (uriboni) wrote :

> Looks good and fixes the issue

Please ignore, commented on the wrong MR

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:1000
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/1756/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/2780/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-amd64-ci/513/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/513/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-i386-ci/513/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/2778/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/webbrowser-app-ci/1756/rebuild

review: Needs Fixing (continuous-integration)
lp:~uriboni/webbrowser-app/find-in-page updated
1001. By Ugo Riboni

Update to match renaming of webview.findInPage to findController

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:1001
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/1787/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/2874/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-amd64-ci/544/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/544/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-i386-ci/544/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/2872/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/webbrowser-app-ci/1787/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Olivier Tilloy (osomon) wrote :

There are a couple of conflicts when merging this branch into the latest trunk, can you please resolve them?

In AddressBar.qml, can the following three new properties be marked readonly?
  findInPagePattern
  current
  count

review: Needs Fixing
lp:~uriboni/webbrowser-app/find-in-page updated
1002. By Ugo Riboni

Merge changes from trunk

1003. By Ugo Riboni

Reorder context menu items to match browser spec more closely

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:1003
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/1809/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/2935/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-amd64-ci/566/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/566/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-i386-ci/566/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/2933/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/webbrowser-app-ci/1809/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Olivier Tilloy (osomon) wrote :

> In AddressBar.qml, can the following three new properties be marked
> readonly?
> findInPagePattern
> current
> count

searchSuggestions.active should be false when chrome.findInPageMode is true.

Is the removal of the 200 response code in HTTPRequestHandler.do_GET for "/suggest" intentional?

In test_activation(), after leaving the find in page mode, can you also check that the contents of the address bar are restored? Also I think the focus should be unset on the address bar when leaving this mode (and this should be tested too). The internal.resetFocus() function in Browser.qml probably is what you want to use.

review: Needs Fixing
lp:~uriboni/webbrowser-app/find-in-page updated
1004. By Ugo Riboni

Merge changes from trunk

1005. By Ugo Riboni

Fix mistake in previous merge from trunk

1006. By Ugo Riboni

Ensure that focus is restored to the webview when we exit find in page mode, and that suggestions are hidden when we enter find in page mode

1007. By Ugo Riboni

Ensure that the address bar text gets back to its correct simplified status when exiting find in page mode

1008. By Ugo Riboni

Add tests for the previous changes

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:1008
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/1823/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/2966/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-amd64-ci/580/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/580/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-i386-ci/580/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/2964/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/webbrowser-app-ci/1823/rebuild

review: Needs Fixing (continuous-integration)
lp:~uriboni/webbrowser-app/find-in-page updated
1009. By Ugo Riboni

Merge translations from trunk

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:1009
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/1834/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/3001/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-amd64-ci/591/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/591/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-i386-ci/591/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/2999/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/webbrowser-app-ci/1834/rebuild

review: Needs Fixing (continuous-integration)
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
lp:~uriboni/webbrowser-app/find-in-page updated
1010. By Ugo Riboni

Remove find in page conditional code for a few things that would be hidden anyway in that mode

1011. By Ugo Riboni

Use an ubuntu color for the counter

1012. By Ugo Riboni

Enable the find in page action only when not already in that mode

1013. By Ugo Riboni

Quiet warnings by using a mock for AddressBar.findController in qml unit tests

1014. By Ugo Riboni

Bump dependency on oxide to 1.8

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:1014
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/1841/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/3025/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-amd64-ci/598/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/598/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-i386-ci/598/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/3023/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/webbrowser-app-ci/1841/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Olivier Tilloy (osomon) wrote :

I am not convinced the change to src/Ubuntu/Web/UbuntuWebView02.qml is needed (I don’t see why it should be, and reverting it doesn’t seem to break the functionality, at least when run locally on my desktop). There’s nothing in the UbuntuWebView component that requires oxide 1.8, it’s only the browser that makes use of it that requires access to the findController API. The import statement in AddressBar.qml should probably be bumped though. Even though not strictly needed for the code to be parsed and interpreted by the QML engine, it’s assuming the findController object is there.

When launching the app locally I’m seeing the following message printed twice before anything else in the console:

    QString::arg: Argument missing: "" , 0

Could you look into what’s causing it to be printed?

lp:~uriboni/webbrowser-app/find-in-page updated
1015. By Ugo Riboni

Bump the version of oxide only where needed

1016. By Ugo Riboni

Simplify code and remove a warning

1017. By Ugo Riboni

Revert change to debian to bump dependency on oxide 1.8 in the browser component

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

That looks good now, thanks!

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

Note: I approved but I’m holding off the top-approval until oxide 1.8 (which this MR has a hard dependency on) is released.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:1017
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/1842/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/3028/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-amd64-ci/599/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/599/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-i386-ci/599/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/3026/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/webbrowser-app-ci/1842/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:1017
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/1873/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/3160/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-amd64-ci/630/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/630/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-i386-ci/630/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/3158/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/webbrowser-app-ci/1873/rebuild

review: Needs Fixing (continuous-integration)
lp:~uriboni/webbrowser-app/find-in-page updated
1018. By Ugo Riboni

Merge changes from trunk

1019. By Ugo Riboni

Leave find in page mode when toggling private browsing on and off

1020. By Ugo Riboni

Enter and Shift+Enter navigate through search results

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:1020
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/1876/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/3173/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-amd64-ci/633/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/633/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-i386-ci/633/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/3171/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/webbrowser-app-ci/1876/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Riccardo Padovani (rpadovani) wrote :

Great feature!
Some comments:
- it's impossible now to enter an address and press enter to navigate (search in page intercept the enter key?)
- I expect CTRL+F to use the search
- How could I esc the search mode? ESC button doesn't work, and clicking the back arrow is a bad UX IMO
- I see it's by design you don't trigger the search if only one character is in the textfield, but it's a bit confusing IMO, because all other browsers highlights only one char, so the first time I entered a char I expect it to be evidenced: if nothing is highlighted, I think there isn't any occurrence of that char in the page

Other than that, looks awesome to me, thanks :-)

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

There are now conflicts when merging this branch into the latest trunk.

Additionally the QML unit tests are failing with errors like this:

    AddressBar.qml:247: TypeError: Property 'next' of object QObject_QML_32(0x2e540a0) is not a function

And as pointed out by Riccardo, validating a URL in the address bar with Enter doesn’t work any longer, and Ctrl+F should activate the find in page mode.

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

Superseded by https://code.launchpad.net/~osomon/webbrowser-app/find-in-page/+merge/263547.

review: Disapprove

Unmerged revisions

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'debian/control'
2--- debian/control 2015-05-27 05:31:12 +0000
3+++ debian/control 2015-06-11 18:28:49 +0000
4@@ -6,7 +6,7 @@
5 debhelper (>= 9),
6 dh-translations,
7 hardening-wrapper,
8- liboxideqt-qmlplugin (>= 1.6),
9+ liboxideqt-qmlplugin (>= 1.8),
10 libqt5sql5-sqlite,
11 python3-all,
12 python3-flake8,
13@@ -34,7 +34,7 @@
14 Depends: ${misc:Depends},
15 ${shlibs:Depends},
16 fonts-liberation,
17- liboxideqt-qmlplugin (>= 1.7),
18+ liboxideqt-qmlplugin (>= 1.8),
19 libqt5sql5-sqlite,
20 qml-module-qt-labs-folderlistmodel,
21 qml-module-qt-labs-settings,
22
23=== added file 'src/app/actions/FindInPage.qml'
24--- src/app/actions/FindInPage.qml 1970-01-01 00:00:00 +0000
25+++ src/app/actions/FindInPage.qml 2015-06-11 18:28:49 +0000
26@@ -0,0 +1,27 @@
27+/*
28+ * Copyright 2015 Canonical Ltd.
29+ *
30+ * This file is part of webbrowser-app.
31+ *
32+ * webbrowser-app is free software; you can redistribute it and/or modify
33+ * it under the terms of the GNU General Public License as published by
34+ * the Free Software Foundation; version 3.
35+ *
36+ * webbrowser-app is distributed in the hope that it will be useful,
37+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
38+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
39+ * GNU General Public License for more details.
40+ *
41+ * You should have received a copy of the GNU General Public License
42+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
43+ */
44+
45+import Ubuntu.Components 1.1
46+import Ubuntu.Unity.Action 1.1 as UnityActions
47+
48+UnityActions.Action {
49+ text: i18n.tr("Find in Page")
50+ // TRANSLATORS: This is a free-form list of keywords associated to the 'Find in Page' action.
51+ // Keywords may actually be sentences, and must be separated by semi-colons.
52+ keywords: i18n.tr("Search in Page")
53+}
54
55=== modified file 'src/app/webbrowser/AddressBar.qml'
56--- src/app/webbrowser/AddressBar.qml 2015-05-26 21:42:54 +0000
57+++ src/app/webbrowser/AddressBar.qml 2015-06-11 18:28:49 +0000
58@@ -19,7 +19,7 @@
59 import QtQuick 2.0
60 import Ubuntu.Components 1.1
61 import Ubuntu.Components.Popups 1.0
62-import com.canonical.Oxide 1.0 as Oxide
63+import com.canonical.Oxide 1.8 as Oxide
64 import ".."
65 import "urlManagement.js" as UrlManagement
66
67@@ -37,6 +37,8 @@
68 signal requestReload()
69 signal requestStop()
70 property string searchUrl
71+ property bool findInPageMode: false
72+ property var findController
73
74 property var securityStatus: null
75
76@@ -48,6 +50,15 @@
77
78 height: textField.height
79
80+ // Only start searches when the user types two or more characters, as
81+ // defined in the design specification for the find in page feature.
82+ Binding {
83+ target: findController
84+ property: "text"
85+ value: textField.text.length > 1 ? textField.text : ""
86+ when: findInPageMode && findController
87+ }
88+
89 TextField {
90 id: textField
91 objectName: "addressBarTextField"
92@@ -59,6 +70,7 @@
93
94 width: iconsRow.width + units.gu(1)
95 height: units.gu(2)
96+ visible: !findInPageMode
97
98 Row {
99 id: iconsRow
100@@ -167,35 +179,54 @@
101 }
102 }
103
104- secondaryItem: Item {
105- id: bookmarkToggle
106- objectName: "bookmarkToggle"
107-
108+ secondaryItem: Row {
109 height: textField.height
110- width: visible ? height : 0
111-
112- visible: internal.idle && addressbar.actualUrl.toString()
113-
114- Icon {
115- height: parent.height - units.gu(2)
116- width: height
117- anchors.centerIn: parent
118-
119- name: addressbar.bookmarked ? "starred" : "non-starred"
120- color: addressbar.bookmarked ? UbuntuColors.orange : keyColor
121+
122+ Label {
123+ objectName: "findInPageCounter"
124+ anchors.verticalCenter: parent.verticalCenter
125+ fontSize: "x-small"
126+ color: UbuntuColors.darkGrey
127+ opacity: findController && findController.count > 0 ? 1.0 : 0.6
128+ visible: findInPageMode
129+
130+ // TRANSLATORS: %2 refers to the total number of find in page results and %1 to the highlighted result
131+ text: i18n.tr("%1/%2").arg(current).arg(count)
132+ property int current: findController ? findController.current : 0
133+ property int count: findController ? findController.count : 0
134 }
135
136- MouseArea {
137- id: bookmarkButton
138- anchors.fill: parent
139- onClicked: addressbar.bookmarked = !addressbar.bookmarked
140+ Item {
141+ id: bookmarkToggle
142+ objectName: "bookmarkToggle"
143+
144+ height: parent.height
145+ width: visible ? height : 0
146+
147+ visible: !findInPageMode && internal.idle && addressbar.actualUrl.toString()
148+
149+ Icon {
150+ height: parent.height - units.gu(2)
151+ width: height
152+ anchors.centerIn: parent
153+
154+ name: addressbar.bookmarked ? "starred" : "non-starred"
155+ color: addressbar.bookmarked ? UbuntuColors.orange : keyColor
156+ }
157+
158+ MouseArea {
159+ id: bookmarkButton
160+ anchors.fill: parent
161+ onClicked: addressbar.bookmarked = !addressbar.bookmarked
162+ }
163 }
164 }
165
166 font.pixelSize: FontUtils.sizeToPixels("small")
167 inputMethodHints: Qt.ImhNoPredictiveText | Qt.ImhUrlCharactersOnly
168
169- placeholderText: i18n.tr("search or enter an address")
170+ placeholderText: findInPageMode ? i18n.tr("find in page")
171+ : i18n.tr("search or enter an address")
172
173 // Work around the "fix" for http://pad.lv/1089370 which
174 // unsets focus on the TextField when it becomes invisible
175@@ -211,6 +242,11 @@
176
177 onAccepted: if (!internal.idle) internal.validate()
178
179+ Keys.onReturnPressed: {
180+ if (event.modifiers & Qt.ShiftModifier) findController.previous()
181+ else findController.next()
182+ }
183+
184 // Make sure that all the text is selected at the first click
185 MouseArea {
186 anchors {
187@@ -291,7 +327,8 @@
188 }
189 }
190
191- onActiveFocusChanged: {
192+ function checkSimplifyUrl() {
193+ if (findInPageMode) return;
194 if (activeFocus) {
195 text = actualUrl
196 } else if (!loading && actualUrl.toString()) {
197@@ -299,6 +336,10 @@
198 }
199 }
200
201+
202+ onActiveFocusChanged: checkSimplifyUrl()
203+ onFindInPageModeChanged: checkSimplifyUrl()
204+
205 onActualUrlChanged: {
206 if (!activeFocus || !actualUrl.toString()) {
207 text = internal.simplifyUrl(actualUrl)
208
209=== modified file 'src/app/webbrowser/Browser.qml'
210--- src/app/webbrowser/Browser.qml 2015-06-02 14:26:38 +0000
211+++ src/app/webbrowser/Browser.qml 2015-06-11 18:28:49 +0000
212@@ -85,6 +85,13 @@
213 Actions.ClearHistory {
214 enabled: browser.historyModel
215 onTriggered: browser.historyModel.clearAll()
216+ },
217+ Actions.FindInPage {
218+ enabled: !chrome.findInPageMode
219+ onTriggered: {
220+ chrome.findInPageMode = true
221+ chrome.focus = true
222+ }
223 }
224 ]
225
226@@ -284,6 +291,8 @@
227 onRemoved: if (chrome.bookmarked && (url === chrome.webview.url)) chrome.bookmarked = false
228 }
229
230+ onFindInPageModeChanged: if (!chrome.findInPageMode) internal.resetFocus()
231+
232 anchors {
233 left: parent.left
234 right: parent.right
235@@ -323,10 +332,11 @@
236 onTriggered: browser.openUrlInNewTab("", true)
237 },
238 Action {
239- objectName: "settings"
240- text: i18n.tr("Settings")
241- iconName: "settings"
242- onTriggered: settingsComponent.createObject(settingsContainer)
243+ objectName: "findinpage"
244+ text: i18n.tr("Find in page")
245+ iconName: "search"
246+ enabled: !chrome.findInPageMode
247+ onTriggered: { chrome.findInPageMode = true; chrome.forceActiveFocus() }
248 },
249 Action {
250 objectName: "privatemode"
251@@ -344,6 +354,12 @@
252 browser.incognito = true
253 }
254 }
255+ },
256+ Action {
257+ objectName: "settings"
258+ text: i18n.tr("Settings")
259+ iconName: "settings"
260+ onTriggered: settingsComponent.createObject(settingsContainer)
261 }
262 ]
263 }
264@@ -358,7 +374,8 @@
265
266 Suggestions {
267 id: suggestionsList
268- opacity: ((chrome.state == "shown") && chrome.activeFocus && (count > 0) && !chrome.drawerOpen) ? 1.0 : 0.0
269+ opacity: ((chrome.state == "shown") && chrome.activeFocus &&
270+ (count > 0) && !chrome.drawerOpen && !chrome.findInPageMode) ? 1.0 : 0.0
271 Behavior on opacity {
272 UbuntuNumberAnimation {}
273 }
274@@ -405,7 +422,7 @@
275 terms: suggestionsList.searchTerms
276 searchEngine: currentSearchEngine
277 active: chrome.activeFocus &&
278- !browser.incognito &&
279+ !browser.incognito && !chrome.findInPageMode &&
280 !UrlManagement.looksLikeAUrl(chrome.text.replace(/ /g, "+"))
281
282 function limit(number) {
283
284=== modified file 'src/app/webbrowser/Chrome.qml'
285--- src/app/webbrowser/Chrome.qml 2015-05-26 21:42:54 +0000
286+++ src/app/webbrowser/Chrome.qml 2015-06-11 18:28:49 +0000
287@@ -29,8 +29,12 @@
288 property list<Action> drawerActions
289 readonly property bool drawerOpen: internal.openDrawer
290 property alias requestedUrl: addressbar.requestedUrl
291+ property bool findInPageMode
292 property alias incognito: addressbar.incognito
293
294+ onFindInPageModeChanged: if (findInPageMode) addressbar.text = ""
295+ onIncognitoChanged: findInPageMode = false
296+
297 backgroundColor: incognito ? UbuntuColors.darkGrey : Theme.palette.normal.background
298
299 FocusScope {
300@@ -57,8 +61,8 @@
301 verticalCenter: parent.verticalCenter
302 }
303
304- enabled: chrome.webview ? chrome.webview.canGoBack : false
305- onTriggered: chrome.webview.goBack()
306+ enabled: chrome.findInPageMode || (chrome.webview ? chrome.webview.canGoBack : false)
307+ onTriggered: chrome.findInPageMode ? (chrome.findInPageMode = false) : chrome.webview.goBack()
308 }
309
310 ChromeButton {
311@@ -78,7 +82,8 @@
312 verticalCenter: parent.verticalCenter
313 }
314
315- enabled: chrome.webview ? chrome.webview.canGoForward : false
316+ enabled: chrome.findInPageMode ? false :
317+ (chrome.webview ? chrome.webview.canGoForward : false)
318 onTriggered: chrome.webview.goForward()
319 }
320
321@@ -86,11 +91,13 @@
322 id: addressbar
323
324 focus: true
325+ findInPageMode: chrome.findInPageMode
326+ findController: chrome.webview ? chrome.webview.findController : null
327
328 anchors {
329 left: forwardButton.right
330 leftMargin: units.gu(1)
331- right: drawerButton.left
332+ right: rightButtonsBar.left
333 rightMargin: units.gu(1)
334 verticalCenter: parent.verticalCenter
335 }
336@@ -100,8 +107,10 @@
337 loading: chrome.webview ? chrome.webview.loading : false
338
339 onValidated: {
340- chrome.webview.forceActiveFocus()
341- chrome.webview.url = requestedUrl
342+ if (!findInPageMode) {
343+ chrome.webview.forceActiveFocus()
344+ chrome.webview.url = requestedUrl
345+ }
346 }
347 onRequestReload: {
348 chrome.webview.forceActiveFocus()
349@@ -127,26 +136,67 @@
350 }
351 }
352
353- ChromeButton {
354- id: drawerButton
355- objectName: "drawerButton"
356-
357- iconName: "contextual-menu"
358- iconSize: 0.5 * height
359- iconColor: internal.iconColor
360-
361- height: chrome.height
362- width: height * 0.8
363+ Row {
364+ id: rightButtonsBar
365
366 anchors {
367 right: parent.right
368 verticalCenter: parent.verticalCenter
369 }
370-
371- onTriggered: {
372- if (!internal.openDrawer) {
373- internal.openDrawer = drawerComponent.createObject(chrome)
374- internal.openDrawer.opened = true
375+ height: parent.height
376+
377+ ChromeButton {
378+ id: findPreviousButton
379+ objectName: "findPreviousButton"
380+
381+ iconName: "up"
382+ iconSize: 0.5 * height
383+
384+ height: chrome.height
385+ width: height * 0.8
386+
387+ anchors.verticalCenter: parent.verticalCenter
388+
389+ visible: findInPageMode
390+ enabled: webview && webview.findController && webview.findController.count > 1
391+ onTriggered: webview.findController.previous()
392+ }
393+
394+ ChromeButton {
395+ id: findNextButton
396+ objectName: "findNextButton"
397+
398+ iconName: "down"
399+ iconSize: 0.5 * height
400+
401+ height: chrome.height
402+ width: height * 0.8
403+
404+ anchors.verticalCenter: parent.verticalCenter
405+
406+ visible: findInPageMode
407+ enabled: webview && webview.findController && webview.findController.count > 1
408+ onTriggered: webview.findController.next()
409+ }
410+
411+ ChromeButton {
412+ id: drawerButton
413+ objectName: "drawerButton"
414+
415+ iconName: "contextual-menu"
416+ iconSize: 0.5 * height
417+ iconColor: internal.iconColor
418+
419+ height: chrome.height
420+ width: height * 0.8
421+
422+ anchors.verticalCenter: parent.verticalCenter
423+
424+ onTriggered: {
425+ if (!internal.openDrawer) {
426+ internal.openDrawer = drawerComponent.createObject(chrome)
427+ internal.openDrawer.opened = true
428+ }
429 }
430 }
431 }
432
433=== modified file 'tests/autopilot/webbrowser_app/emulators/browser.py'
434--- tests/autopilot/webbrowser_app/emulators/browser.py 2015-05-28 13:56:52 +0000
435+++ tests/autopilot/webbrowser_app/emulators/browser.py 2015-06-11 18:28:49 +0000
436@@ -205,6 +205,14 @@
437 return drawer.select_single("AbstractButton", objectName=actionName,
438 visible=True)
439
440+ def get_find_next_button(self):
441+ return self.select_single("ChromeButton",
442+ objectName="findNextButton")
443+
444+ def get_find_prev_button(self):
445+ return self.select_single("ChromeButton",
446+ objectName="findPreviousButton")
447+
448
449 class AddressBar(uitk.UbuntuUIToolkitCustomProxyObjectBase):
450
451@@ -238,6 +246,9 @@
452 def get_bookmark_toggle(self):
453 return self.select_single("QQuickItem", objectName="bookmarkToggle")
454
455+ def get_find_in_page_counter(self):
456+ return self.select_single("Label", objectName="findInPageCounter")
457+
458
459 class Suggestions(uitk.UbuntuUIToolkitCustomProxyObjectBase):
460
461
462=== modified file 'tests/autopilot/webbrowser_app/tests/http_server.py'
463--- tests/autopilot/webbrowser_app/tests/http_server.py 2015-04-23 15:34:16 +0000
464+++ tests/autopilot/webbrowser_app/tests/http_server.py 2015-06-11 18:28:49 +0000
465@@ -132,6 +132,11 @@
466 html += '<div style="height: 100%"></div>'
467 html += '</a></body></html>'
468 self.send_html(html)
469+ elif self.path == "/findinpage":
470+ # send a page with some searchable text
471+ self.send_response(200)
472+ html = '<html><body>hello this is text and more text</body></html>'
473+ self.send_html(html)
474 elif self.path.startswith("/suggest"):
475 self.send_response(200)
476 self.send_header("Content-Type", "text/x-suggestions+json")
477
478=== added file 'tests/autopilot/webbrowser_app/tests/test_findinpage.py'
479--- tests/autopilot/webbrowser_app/tests/test_findinpage.py 1970-01-01 00:00:00 +0000
480+++ tests/autopilot/webbrowser_app/tests/test_findinpage.py 2015-06-11 18:28:49 +0000
481@@ -0,0 +1,134 @@
482+# -*- Mode: Python; coding: utf-8; indent-tabs-mode: nil; tab-width: 4 -*-
483+#
484+# Copyright 2015 Canonical
485+#
486+# This program is free software: you can redistribute it and/or modify it
487+# under the terms of the GNU General Public License version 3, as published
488+# by the Free Software Foundation.
489+#
490+# This program is distributed in the hope that it will be useful,
491+# but WITHOUT ANY WARRANTY; without even the implied warranty of
492+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
493+# GNU General Public License for more details.
494+#
495+# You should have received a copy of the GNU General Public License
496+# along with this program. If not, see <http://www.gnu.org/licenses/>.
497+
498+from autopilot.matchers import Eventually
499+from testtools.matchers import Equals
500+
501+from webbrowser_app.tests import StartOpenRemotePageTestCaseBase
502+
503+
504+class TestFindInPage(StartOpenRemotePageTestCaseBase):
505+
506+ """Tests the find in page functionality."""
507+
508+ def setUp(self):
509+ super().setUp()
510+ self.chrome = self.main_window.chrome
511+ self.url = self.base_url + "/findinpage"
512+
513+ def activate_find_in_page(self, navigateFirst=True):
514+ if navigateFirst:
515+ self.main_window.go_to_url(self.url)
516+ self.main_window.wait_until_page_loaded(self.url)
517+
518+ drawer_button = self.chrome.get_drawer_button()
519+ self.pointing_device.click_object(drawer_button)
520+ self.chrome.get_drawer()
521+ action = self.chrome.get_drawer_action("findinpage")
522+ self.pointing_device.click_object(action)
523+
524+ def test_activation(self):
525+ next = self.chrome.get_find_next_button()
526+ prev = self.chrome.get_find_prev_button()
527+ bar = self.chrome.address_bar
528+ counter = bar.get_find_in_page_counter()
529+
530+ self.assertThat(bar.findInPageMode, Equals(False))
531+ self.assertThat(next.visible, Equals(False))
532+ self.assertThat(prev.visible, Equals(False))
533+ self.assertThat(counter.visible, Equals(False))
534+ self.assertThat(bar.activeFocus, Equals(False))
535+
536+ previous_text = bar.text
537+
538+ self.activate_find_in_page(False)
539+ self.assertThat(bar.findInPageMode, Eventually(Equals(True)))
540+ self.assertThat(next.visible, Eventually(Equals(True)))
541+ self.assertThat(prev.visible, Eventually(Equals(True)))
542+ self.assertThat(counter.visible, Eventually(Equals(True)))
543+ self.assertThat(bar.activeFocus, Eventually(Equals(True)))
544+
545+ self.assertThat(self.chrome.is_back_button_enabled(), Equals(True))
546+ self.assertThat(self.chrome.is_forward_button_enabled(), Equals(False))
547+ self.chrome.go_back()
548+
549+ self.assertThat(bar.findInPageMode, Eventually(Equals(False)))
550+ self.assertThat(next.visible, Eventually(Equals(False)))
551+ self.assertThat(prev.visible, Eventually(Equals(False)))
552+ self.assertThat(counter.visible, Eventually(Equals(False)))
553+ self.assertThat(bar.activeFocus, Eventually(Equals(False)))
554+ self.assertThat(bar.text, Eventually(Equals(previous_text)))
555+
556+ def test_counter(self):
557+ bar = self.chrome.address_bar
558+ counter = bar.get_find_in_page_counter()
559+
560+ self.activate_find_in_page()
561+ bar.write("text")
562+ self.assertThat(counter.current, Eventually(Equals(1)))
563+ self.assertThat(counter.count, Eventually(Equals(2)))
564+
565+ bar.write("hello")
566+ self.assertThat(counter.current, Eventually(Equals(1)))
567+ self.assertThat(counter.count, Eventually(Equals(1)))
568+
569+ bar.write("")
570+ self.assertThat(counter.current, Eventually(Equals(0)))
571+ self.assertThat(counter.count, Eventually(Equals(0)))
572+
573+ def test_minimum_two_characters(self):
574+ bar = self.chrome.address_bar
575+ counter = bar.get_find_in_page_counter()
576+
577+ self.activate_find_in_page()
578+ bar.write("t")
579+ self.assertThat(counter.current, Eventually(Equals(0)))
580+ self.assertThat(counter.count, Eventually(Equals(0)))
581+
582+ bar.write("e", False)
583+ self.assertThat(counter.current, Eventually(Equals(1)))
584+ self.assertThat(counter.count, Eventually(Equals(2)))
585+
586+ def test_navigation(self):
587+ bar = self.chrome.address_bar
588+ counter = bar.get_find_in_page_counter()
589+ next = self.chrome.get_find_next_button()
590+ prev = self.chrome.get_find_prev_button()
591+
592+ self.activate_find_in_page()
593+ self.assertThat(next.enabled, Eventually(Equals(False)))
594+ self.assertThat(prev.enabled, Eventually(Equals(False)))
595+
596+ bar.write("text")
597+ self.assertThat(next.enabled, Eventually(Equals(True)))
598+ self.assertThat(prev.enabled, Eventually(Equals(True)))
599+
600+ self.pointing_device.click_object(next)
601+ self.assertThat(counter.current, Eventually(Equals(2)))
602+ self.assertThat(counter.count, Eventually(Equals(2)))
603+ self.pointing_device.click_object(next)
604+ self.assertThat(counter.current, Eventually(Equals(1)))
605+ self.assertThat(counter.count, Eventually(Equals(2)))
606+ self.pointing_device.click_object(prev)
607+ self.assertThat(counter.current, Eventually(Equals(2)))
608+ self.assertThat(counter.count, Eventually(Equals(2)))
609+ self.pointing_device.click_object(next)
610+ self.assertThat(counter.current, Eventually(Equals(1)))
611+ self.assertThat(counter.count, Eventually(Equals(2)))
612+
613+ bar.write("hello")
614+ self.assertThat(next.enabled, Eventually(Equals(False)))
615+ self.assertThat(prev.enabled, Eventually(Equals(False)))
616
617=== modified file 'tests/unittests/qml/tst_AddressBar.qml'
618--- tests/unittests/qml/tst_AddressBar.qml 2015-05-18 13:53:05 +0000
619+++ tests/unittests/qml/tst_AddressBar.qml 2015-06-11 18:28:49 +0000
620@@ -39,6 +39,10 @@
621 height: parent.height / 2
622
623 searchUrl: "http://www.ubuntu.com/search?q={searchTerms}"
624+ findController: QtObject {
625+ property int current
626+ property int count
627+ }
628 }
629
630 // only exists to steal focus from the address bar
631@@ -363,5 +367,16 @@
632 clickItem(addressBar)
633 compare(addressBar.text, url)
634 }
635+
636+ function test_exitingFindInPageRestoresUrl() {
637+ var url = "http://example.org/"
638+ addressBar.actualUrl = url
639+ addressBar.findInPageMode = true
640+ verify(addressBar.activeFocus)
641+ compare(addressBar.text, "")
642+ typeString("hello")
643+ addressBar.findInPageMode = false
644+ compare(addressBar.text, url)
645+ }
646 }
647 }

Subscribers

People subscribed via source and target branches

to status/vote changes: