Merge lp:~uriboni/webbrowser-app/addressbar-load-no-clear into lp:webbrowser-app

[Ugo Riboni]

Prevent the address bar from being cleared when the actual url changes and the user has already started typing.

Prevent the address bar from being cleared when the actual url changes and the user has already started typing

[lp:~uriboni/webbrowser-app/addressbar-load-no-clear]

1247. By Ugo Riboni on 2015-10-22

Transform an unit test into an AP test that does not fail with this change and actually tests the bug fix it is supposed to test as described in the bug report

1248. By Ugo Riboni on 2015-10-22

Add a new AP test to verify this bug fix

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:1246
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/2391/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/4769/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-amd64-ci/1145/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/1145/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-i386-ci/1145/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/4766/console

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

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:1248
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/2394/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/4777
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-amd64-ci/1148
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/1148
        deb: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/1148/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-i386-ci/1148
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-vivid-mako/3850
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/4774
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/4774/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/24481

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

review: Needs Fixing (continuous-integration)

[Olivier Tilloy (osomon)]

test_clears_when_actual_url_changed doesn’t actually test bug #1456199, it should focus the address bar before opening a new tab.

review: Needs Fixing

[Olivier Tilloy (osomon)]

And a link to bug #1487713 in a comment before test_does_not_clear_when_typing_while_loading would be useful.

[Olivier Tilloy (osomon)]

The change otherwise looks good, I’ll play further with it on my device to convince myself that it doesn’t introduce any regression (this code is not exactly as robust as I’d like it to be, despite the unit tests and autopilot tests).

[lp:~uriboni/webbrowser-app/addressbar-load-no-clear]

1249. By Ugo Riboni on 2015-10-26

Adjust AP test to more closely match what the bug report says

1250. By Ugo Riboni on 2015-10-26

Reference bug that the new AP test is for

1251. By Ugo Riboni on 2015-10-26

Sync with trunk

1252. By Ugo Riboni on 2015-10-26

Clear the address bar when going into incognito mode

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:1251
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/2401/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/4817
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-amd64-ci/1155
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/1155
        deb: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/1155/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-i386-ci/1155
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-vivid-mako/3880
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/4814
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/4814/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/24557

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

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:1252
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/2403/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/4831
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-amd64-ci/1157
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/1157
        deb: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/1157/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-i386-ci/1157
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-vivid-mako/3889
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/4828
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/4828/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/24576

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

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:1252
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/2406/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/4844
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-amd64-ci/1160
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/1160
        deb: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/1160/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-i386-ci/1160
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-vivid-mako/3901
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/4841
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/4841/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/24597

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

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:1252
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/2412/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/4861
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-amd64-ci/1166
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/1166
        deb: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/1166/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-i386-ci/1166
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-vivid-mako/3913
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/4858
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/4858/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/24619

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

review: Needs Fixing (continuous-integration)

[lp:~uriboni/webbrowser-app/addressbar-load-no-clear]

1253. By Ugo Riboni on 2015-10-29

Merge changes from trunk

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:1253
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/2421/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/4905
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-amd64-ci/1175
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/1175
        deb: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/1175/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-i386-ci/1175
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-vivid-mako/3946
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/4902
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/4902/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/24685

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

review: Needs Fixing (continuous-integration)

[Olivier Tilloy (osomon)]

Is revision 1252 ("Clear the address bar when going into incognito mode") really needed? I would have expected the address bar to be cleared anyway, given that a new tab is being open. Does this code fix an existing issue? If so there should probably be an additional autopilot test for it.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:1253
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/2426/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/4936
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-amd64-ci/1180
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/1180
        deb: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/1180/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-i386-ci/1180
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-vivid-mako/3965
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/4933
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/4933/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/24734

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

review: Needs Fixing (continuous-integration)

[Ugo Riboni (uriboni)]

> Is revision 1252 ("Clear the address bar when going into incognito mode")
> really needed? I would have expected the address bar to be cleared anyway,
> given that a new tab is being open. Does this code fix an existing issue? If
> so there should probably be an additional autopilot test for it.

Not clearing the address bar when going incognito was a side effect of the changes I made, so I had to fix that.
There was already an AP test for it (the first in test_private.py)

[Olivier Tilloy (osomon)]

Got it, thanks.

webbrowser_app.tests.test_addressbar_states.TestAddressBarStates.test_clears_when_actual_url_changed seems to be failing reliably in the last few CI runs on mako, can you check what’s going on?

review: Needs Fixing

[lp:~uriboni/webbrowser-app/addressbar-load-no-clear]

1254. By Ugo Riboni on 2015-11-03

Fix failing AP test

1255. By Ugo Riboni on 2015-11-03

The test was actually OK, but it should have run only on desktop

[Ugo Riboni (uriboni)]

> Got it, thanks.
>
> webbrowser_app.tests.test_addressbar_states.TestAddressBarStates.test_clears_w
> hen_actual_url_changed seems to be failing reliably in the last few CI runs on
> mako, can you check what’s going on?

It does fail because the test is for http://pad.lv/1456199 which is explicitly a desktop only problem. I excluded that test from running on devices because it does not make sense there.

[Olivier Tilloy (osomon)]

LGTM, thanks.

review: Approve

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1255
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/2434/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/4990
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-amd64-ci/1188
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/1188
        deb: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/1188/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-i386-ci/1188
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-vivid-mako/4008
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/4987
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/4987/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/24850

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

review: Approve (continuous-integration)

=== modified file 'src/app/webbrowser/AddressBar.qml'
--- src/app/webbrowser/AddressBar.qml	2015-09-01 17:25:21 +0000
+++ src/app/webbrowser/AddressBar.qml	2015-11-03 09:28:15 +0000
@@ -348,6 +348,13 @@
         property bool simplified: false
     }
 
+    onIncognitoChanged: {
+        if (incognito) {
+            text = ""
+            internal.simplified = false
+        }
+    }
+
     onEditingChanged: {
         if (findInPageMode) return
         if (editing && internal.simplified) {
@@ -376,7 +383,7 @@
     }
 
     onActualUrlChanged: {
-        if ((editing && actualUrl.toString()) || findInPageMode) return
+        if (editing || findInPageMode) return
         if (canSimplifyText) {
             text = internal.simplifyUrl(actualUrl)
             internal.simplified = true

=== modified file 'tests/autopilot/webbrowser_app/tests/test_addressbar_states.py'
--- tests/autopilot/webbrowser_app/tests/test_addressbar_states.py	2015-02-17 20:52:30 +0000
+++ tests/autopilot/webbrowser_app/tests/test_addressbar_states.py	2015-11-03 09:28:15 +0000
@@ -14,6 +14,11 @@
 # You should have received a copy of the GNU General Public License
 # along with this program. If not, see <http://www.gnu.org/licenses/>.
 
+import testtools
+from testtools.matchers import Equals
+from autopilot.matchers import Eventually
+from autopilot.platform import model
+
 from webbrowser_app.tests import StartOpenRemotePageTestCaseBase
 
 
@@ -53,3 +58,29 @@
         address_bar.write(self.url)
         address_bar.click_action_button()
         address_bar.activeFocus.wait_for(False)
+
+    # http://pad.lv/1456199
+    @testtools.skipIf(model() != "Desktop", "on desktop only")
+    def test_clears_when_actual_url_changed(self):
+        address_bar = self.main_window.address_bar
+        self.pointing_device.click_object(address_bar)
+        address_bar.activeFocus.wait_for(True)
+        url = self.base_url + "/test1"
+        self.main_window.go_to_url(url)
+        self.main_window.wait_until_page_loaded(url)
+        self.pointing_device.click_object(address_bar)
+        address_bar.activeFocus.wait_for(True)
+        self.new_tab_view = self.open_new_tab(open_tabs_view=True)
+        self.assertThat(address_bar.text, Eventually(Equals("")))
+
+    # http://pad.lv/1487713
+    def test_does_not_clear_when_typing_while_loading(self):
+        address_bar = self.main_window.address_bar
+        self.pointing_device.click_object(address_bar)
+        address_bar.activeFocus.wait_for(True)
+        url = self.base_url + "/wait/3"
+        self.main_window.go_to_url(url)
+        self.pointing_device.click_object(address_bar)
+        address_bar.write("x")
+        self.main_window.wait_until_page_loaded(url)
+        self.assertThat(address_bar.text, Equals("x"))

=== modified file 'tests/unittests/qml/tst_AddressBar.qml'
--- tests/unittests/qml/tst_AddressBar.qml	2015-08-11 10:37:56 +0000
+++ tests/unittests/qml/tst_AddressBar.qml	2015-11-03 09:28:15 +0000
@@ -274,19 +274,6 @@
             compare(addressBar.text, data.actualUrl)
         }
 
-        function test_shouldBeClearedWhenFocusedIfActualUrlIsCleared() {
-            // https://launchpad.net/bugs/1456199
-            var text = "http://example.org"
-            typeString(text)
-            compare(addressBar.text, text)
-            verify(addressBar.activeFocus)
-            addressBar.actualUrl = text
-            verify(addressBar.activeFocus)
-            addressBar.actualUrl = ""
-            verify(addressBar.activeFocus)
-            compare(addressBar.text, "")
-        }
-
         function test_actionButtonShouldBeDisabledWhenEmpty() {
             verify(!addressBar.__actionButton.enabled)
             keyClick(Qt.Key_U)