Merge lp:~michael-sheldon/webbrowser-app/simplify-address-bar into lp:webbrowser-app

[Michael Sheldon]

Simplify the address displayed in the address bar when not being edited

Simplify the address displayed in the address bar when not being edited

[Michael Sheldon (michael-sheldon)]

Are there any related MPs required for this MP to build/function as expected? Please list.

 * No

Is your branch in sync with latest trunk (e.g. bzr pull lp:trunk -> no changes)

 * Yes

Did you perform an exploratory manual test run of your code change and any related functionality on device or emulator?

 * Yes

Did you successfully run all tests found in your component's Test Plan (https://wiki.ubuntu.com/Process/Merges/TestPlan/webbrowser-app) on device or emulator?

 * Yes

If you changed the UI, was the change specified/approved by design?

 * Yes

If you changed UI labels, did you update the pot file?

 * No change

If you changed the packaging (debian), did you add a core-dev as a reviewer to this MP?

 * No change

[lp:~michael-sheldon/webbrowser-app/simplify-address-bar]

598. By Michael Sheldon on 2014-08-27

Remove accidentally added merge file

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:598
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/1065/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-utopic-touch/4110
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-utopic/3139/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-utopic-amd64-ci/264
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-utopic-armhf-ci/264
        deb: http://jenkins.qa.ubuntu.com/job/webbrowser-app-utopic-armhf-ci/264/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-utopic-i386-ci/264
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-mako/3943
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/5362
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/5362/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/12200
    FAILURE: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-utopic/2566/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/3426
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/3426/artifact/work/output/*zip*/output.zip

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

review: Needs Fixing (continuous-integration)

[Olivier Tilloy (osomon)]

132 + {text: "ubuntu.com", url: "http://user:<email address hidden>"},
133 + {text: "ubuntu.com", url: "http://user:<email address hidden>"},

this test row is duplicated

[Olivier Tilloy (osomon)]

In simplifyUrl(…):

 - there are several calls to url.toString(), you might want to define a var that contains its value

 - if(hasProtocol) : missing whitespace before the opening parenthesis

 - domain.replace("www.", "") : this should happen only if "www." is at the beginning of the domain (e.g. "http://www.com" should not return "com" as a domain, you could add a test case for that)

review: Needs Fixing

[Olivier Tilloy (osomon)]

Could you please add an autopilot test to verify that focusing / unfocusing / focusing again the address bar updates its contents as expected?

[lp:~michael-sheldon/webbrowser-app/simplify-address-bar]

599. By Michael Sheldon on 2014-08-27

Reduce number of toString calls when simplifying URL and ensure 'www.' only get replaced at the beginning of a URL

600. By Michael Sheldon on 2014-08-27

Remove duplicate test case

601. By Michael Sheldon on 2014-08-27

Add test for URLs containing 'www.' in the middle of the domain

602. By Michael Sheldon on 2014-08-27

Add check for urls of the form http://www.com when simplifying domains

603. By Michael Sheldon on 2014-08-28

Add autopilot test for url simplification when focusing, defocusing and refocusing the addressbar

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:603
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/1067/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-utopic-touch/4185
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-utopic/3175
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-utopic-amd64-ci/266
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-utopic-armhf-ci/266
        deb: http://jenkins.qa.ubuntu.com/job/webbrowser-app-utopic-armhf-ci/266/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-utopic-i386-ci/266
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-mako/4000
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/5437
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/5437/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/12275
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-utopic/2594
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/3462
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/3462/artifact/work/output/*zip*/output.zip

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

review: Approve (continuous-integration)

[Bill Filler (bfiller)]

Seems to work well except in one case and I don't know what the intended design is.
If you load a page, you'll see the simplified url. Then if you click in the field you'll see the expanded url which is correct. But then clicking outside of the url field I would expect the url to show it's simplified form again, but it does not. It shows long form until you reload the page again.

review: Needs Fixing

[Bill Filler (bfiller)]

I can't always reproduce the problem, but is was happening for a while. Other thing I noticed when in that state is that selecting the field would not cause the Refresh button to be displayed so there was no way to refresh the page.

[lp:~michael-sheldon/webbrowser-app/simplify-address-bar]

604. By Michael Sheldon on 2014-09-08

Merge from trunk

605. By Michael Sheldon on 2014-09-08

Allow address bar to return to simplified URL whilst still loading

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:605
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/1079/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-utopic-touch/4594
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-utopic/3401/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-utopic-amd64-ci/278
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-utopic-armhf-ci/278
        deb: http://jenkins.qa.ubuntu.com/job/webbrowser-app-utopic-armhf-ci/278/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-utopic-i386-ci/278
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-mako/4377
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/5846
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/5846/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/12864
    FAILURE: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-utopic/2805/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/3703
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/3703/artifact/work/output/*zip*/output.zip

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

review: Needs Fixing (continuous-integration)

[Bill Filler (bfiller)]

Did you perform an exploratory manual test run of the code change and any related functionality on device or emulator?
yes

Did you successfully run all tests found in your component's Test Plan (https://wiki.ubuntu.com/Process/Merges/TestPlan/<package-name>) on device or emulator?
yes

Did CI run pass? If not, please explain why.
yes

Have you checked that submitter has accurately filled out the submitter checklist and has taken no shortcut?
yes

review: Approve

=== modified file 'src/app/webbrowser/AddressBar.qml'
--- src/app/webbrowser/AddressBar.qml	2014-08-13 22:04:39 +0000
+++ src/app/webbrowser/AddressBar.qml	2014-09-08 10:11:06 +0000
@@ -34,6 +34,7 @@
     signal requestStop()
     signal pressAndHold()
     property string searchUrl
+    signal textFieldFocused()
 
     height: textField.height
 
@@ -161,24 +162,13 @@
 
         onAccepted: if (addressbar.state != "") parent.validate()
 
-        function ensureSchemeVisibleWhenUnfocused() {
-            // Ensure the beginning of the URL is always visible when unfocused.
-            // In the future, we’ll have a smarter address bar that hides the
-            // scheme to save some extra space and display more of the
-            // meaningful part of the URL (domain name and path).
-            if (!activeFocus) {
-                cursorPosition = 0
-            }
-        }
         onActiveFocusChanged: {
-            if (!activeFocus) {
-                if (!addressbar.loading && addressbar.actualUrl.toString()) {
-                    text = addressbar.actualUrl
-                }
+            if (activeFocus) {
+                addressbar.textFieldFocused();
+            } else if (addressbar.actualUrl.toString()) {
+                text = addressbar.simplifyUrl(addressbar.actualUrl)
             }
-            ensureSchemeVisibleWhenUnfocused()
         }
-        onTextChanged: ensureSchemeVisibleWhenUnfocused()
 
         // Make sure that all the text is selected at the first click
         MouseArea {
@@ -249,5 +239,35 @@
         validated()
     }
 
-    onActualUrlChanged: text = actualUrl
+    function simplifyUrl(url) {
+        var urlString = url.toString();
+        var hasProtocol = urlString.indexOf("://") != -1
+        var domain;
+        if (hasProtocol) {
+            if (urlString.split("://")[0] == "file") {
+                // Don't process file:// urls
+                return url;
+            }
+            domain = urlString.split('/')[2];
+        } else {
+            domain = urlString.split('/')[0];
+        }
+        if (typeof domain !== 'undefined' && domain.length > 0) {
+            // Remove user component if present
+            var userRemoved = domain.split('@')[1];
+            if (typeof userRemoved !== 'undefined') {
+                domain = userRemoved;
+            }
+            // Remove port number if present
+            domain = domain.split(':')[0];
+            if (domain.lastIndexOf('.') != 3) { // http://www.com shouldn't be trimmed
+                domain = domain.replace(/^www\./, "");
+            }
+            return domain;
+        } else {
+            return url;
+        }
+    }
+
+    onActualUrlChanged: text = simplifyUrl(actualUrl)
 }

=== modified file 'src/app/webbrowser/Chrome.qml'
--- src/app/webbrowser/Chrome.qml	2014-08-18 09:44:55 +0000
+++ src/app/webbrowser/Chrome.qml	2014-09-08 10:11:06 +0000
@@ -107,6 +107,7 @@
             onValidated: chrome.webview.url = requestedUrl
             onRequestReload: chrome.webview.reload()
             onRequestStop: chrome.webview.stop()
+            onTextFieldFocused: text = chrome.webview.url
 
             Connections {
                 target: chrome.webview

=== modified file 'tests/autopilot/webbrowser_app/tests/__init__.py'
--- tests/autopilot/webbrowser_app/tests/__init__.py	2014-08-20 10:42:40 +0000
+++ tests/autopilot/webbrowser_app/tests/__init__.py	2014-09-08 10:11:06 +0000
@@ -186,6 +186,7 @@
         self.server = http_server.HTTPServerInAThread()
         self.addCleanup(self.server.cleanup)
         self.base_url = "http://localhost:{}".format(self.server.port)
+        self.domain = "localhost"
         self.ping_server()
         self.url = self.base_url + "/loremipsum"
         self.ARGS = self.ARGS + [self.url]

=== modified file 'tests/autopilot/webbrowser_app/tests/test_addressbar_states.py'
--- tests/autopilot/webbrowser_app/tests/test_addressbar_states.py	2014-08-13 22:04:39 +0000
+++ tests/autopilot/webbrowser_app/tests/test_addressbar_states.py	2014-09-08 10:11:06 +0000
@@ -51,12 +51,12 @@
 
     def test_url_reset_when_unfocused(self):
         address_bar = self.main_window.get_chrome().get_address_bar()
-        self.assertThat(address_bar.text, Eventually(Equals(self.url)))
+        self.assertThat(address_bar.text, Eventually(Equals(self.domain)))
         self.clear_address_bar()
         self.assertThat(address_bar.text, Eventually(Equals("")))
         webview = self.main_window.get_current_webview()
         self.pointing_device.click_object(webview)
-        self.assertThat(address_bar.text, Eventually(Equals(self.url)))
+        self.assertThat(address_bar.text, Eventually(Equals(self.domain)))
 
     def test_looses_focus_when_loading_starts(self):
         address_bar = self.main_window.get_chrome().get_address_bar()
@@ -77,3 +77,13 @@
         self.pointing_device.click_object(action_button)
         self.assertThat(address_bar.state, Eventually(Equals("")))
         self.assertThat(address_bar.activeFocus, Eventually(Equals(False)))
+
+    def test_url_simplification(self):
+        address_bar = self.main_window.get_chrome().get_address_bar()
+        self.pointing_device.click_object(address_bar)
+        self.assertThat(address_bar.text, Eventually(Equals(self.url)))
+        webview = self.main_window.get_current_webview()
+        self.pointing_device.click_object(webview)
+        self.assertThat(address_bar.text, Eventually(Equals(self.domain)))
+        self.pointing_device.click_object(address_bar)
+        self.assertThat(address_bar.text, Eventually(Equals(self.url)))

=== modified file 'tests/unittests/qml/tst_AddressBar.qml'
--- tests/unittests/qml/tst_AddressBar.qml	2014-06-30 11:01:27 +0000
+++ tests/unittests/qml/tst_AddressBar.qml	2014-09-08 10:11:06 +0000
@@ -113,6 +113,27 @@
         compare(addressBar.requestedUrl, data.requestedUrl)
     }
 
+    function test_simplify_data() {
+        return [
+            {text: "ubuntu.com", url: "http://www.ubuntu.com"},
+            {text: "ubuntu.com", url: "http://www.ubuntu.com/"},
+            {text: "ubuntu.com", url: "http://www.ubuntu.com:80"},
+            {text: "ubuntuwww.com", url: "http://www.ubuntuwww.com"},
+            {text: "www.com", url: "http://www.com"},
+            {text: "ubuntu.com", url: "http://[e-mail]"},
+            {text: "ubuntu.com", url: "http://user:[e-mail]"},
+            {text: "ubuntu.com", url: "http://user:[e-mail]:80"},
+            {text: "file:///home/phablet/", url: "file:///home/phablet/"},
+            {text: "en.wikipedia.org", url: "http://en.wikipedia.org/wiki/Ubuntu"},
+            {text: "en.wikipedia.org", url: "en.wikipedia.org"},
+            {text: "en.wikipedia.org", url: "en.wikipedia.org/wiki/Foo"}
+        ]
+    }
+
+    function test_simplify(data) {
+        compare(addressBar.simplifyUrl(data.url), data.text);
+    }
+
     AddressBar {
         id: addressBar
         searchUrl: "http://www.ubuntu.com/search?q={searchTerms}"