Merge lp:~michael-sheldon/webbrowser-app/fix-1354388 into lp:webbrowser-app

[Michael Sheldon]

Ensure that the url of the page we're loading is displayed, instead of the currently loaded url.

Ensure that the url of the page we're loading is displayed, instead of the currently loaded url.

[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?

 * No change

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

[Olivier Tilloy (osomon)]

Looks good, and works as expected. Thanks!

I’m seeing an additional use case that doesn’t seem to be covered (although I don’t think it’s a new regression, so it may be fine to tackle it separately):

  - browse to any page that triggers the display of a certificate warning page (e.g. https://testssl-expire.disig.sk/) and choose to proceed anyway
  - close the browser and open it again, the previously open page is re-loaded
  - the certificate warning page is displayed, but the address bar is empty, until you choose to proceed anyway

If you think it can be addressed easily and with minimal changes to this MR, then go ahead, otherwise I’m happy to report it separately afterwards.

review: Approve

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:732
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/1101/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-utopic-touch/5136
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-utopic/3742
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-utopic-amd64-ci/300
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-utopic-armhf-ci/300
        deb: http://jenkins.qa.ubuntu.com/job/webbrowser-app-utopic-armhf-ci/300/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-utopic-i386-ci/300
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-mako/4885
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/6388
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/6388/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/13677
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-utopic/3132
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/4053
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/4053/artifact/work/output/*zip*/output.zip

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

review: Approve (continuous-integration)

[lp:~michael-sheldon/webbrowser-app/fix-1354388]

734. By Michael Sheldon on 2014-09-24

When a certificate error is displayed set that page's URL in the address bar to ensure we show the URL if a page with an error is loaded at start up

[Olivier Tilloy (osomon)]

That seems to work well, however I’m wondering if there isn’t a more general way to fix that, because in the case where there is no certificate error, opening the browser on a previously open page will still display a blank address bar for a short while.

Basically we need to set chrome.requestedUrl accordingly when the currently active webview starts loading, regardless of whether there is a certificate error. Not sure exactly where in the code this needs to be done though, I’d need to take a closer look.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:734
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/1103/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-utopic-touch/5149
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-utopic/3755
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-utopic-amd64-ci/302
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-utopic-armhf-ci/302
        deb: http://jenkins.qa.ubuntu.com/job/webbrowser-app-utopic-armhf-ci/302/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-utopic-i386-ci/302
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-mako/4894
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/6401
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/6401/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/13691
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-utopic/3143
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/4066
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/4066/artifact/work/output/*zip*/output.zip

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

review: Approve (continuous-integration)

[lp:~michael-sheldon/webbrowser-app/fix-1354388]

735. By Michael Sheldon on 2014-09-25

Display the URL in the address bar as soon as a tab is created

[Olivier Tilloy (osomon)]

19 + if(actualUrl.length > 0) {

actualUrl is of type url, which doesn’t have a 'length' property, so actualUrl.length will always return undefined. You’ll need to do "actualUrl.toString().length".

review: Needs Fixing

[lp:~michael-sheldon/webbrowser-app/fix-1354388]

736. By Michael Sheldon on 2014-09-25

Fix check for empty string

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:735
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/1106/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-utopic-touch/5177
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-utopic/3778
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-utopic-amd64-ci/305
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-utopic-armhf-ci/305
        deb: http://jenkins.qa.ubuntu.com/job/webbrowser-app-utopic-armhf-ci/305/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-utopic-i386-ci/305
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-mako/4922
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/6429
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/6429/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/13745
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-utopic/3164
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/4089
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/4089/artifact/work/output/*zip*/output.zip

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

review: Needs Fixing (continuous-integration)

[Olivier Tilloy (osomon)]

Looks good now, thanks!

review: Approve

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:736
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/1110/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-utopic-touch/5211
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-utopic/3798
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-utopic-amd64-ci/309
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-utopic-armhf-ci/309
        deb: http://jenkins.qa.ubuntu.com/job/webbrowser-app-utopic-armhf-ci/309/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-utopic-i386-ci/309
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-mako/4945
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/6463
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/6463/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/13787
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-utopic/3183
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/4109
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/4109/artifact/work/output/*zip*/output.zip

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

review: Approve (continuous-integration)

=== modified file 'src/app/webbrowser/AddressBar.qml'
--- src/app/webbrowser/AddressBar.qml	2014-09-25 12:34:49 +0000
+++ src/app/webbrowser/AddressBar.qml	2014-09-25 12:34:51 +0000
@@ -351,7 +351,7 @@
         onActiveFocusChanged: {
             if (activeFocus) {
                 addressbar.textFieldFocused();
-            } else if (addressbar.actualUrl.toString()) {
+            } else if (!addressbar.loading && addressbar.actualUrl.toString()) {
                 text = addressbar.simplifyUrl(addressbar.actualUrl)
             }
         }
@@ -455,5 +455,10 @@
         }
     }
 
-    onActualUrlChanged: text = simplifyUrl(actualUrl)
+    onActualUrlChanged: {
+        if(actualUrl.toString().length > 0) {
+            text = simplifyUrl(actualUrl)
+        }
+    }
+    onRequestedUrlChanged: text = simplifyUrl(requestedUrl)
 }
 
=== modified file 'src/app/webbrowser/Browser.qml'
--- src/app/webbrowser/Browser.qml	2014-09-25 12:34:49 +0000
+++ src/app/webbrowser/Browser.qml	2014-09-25 12:34:51 +0000
@@ -252,6 +252,7 @@
             onSelected: {
                 browser.currentWebview.url = url
                 browser.currentWebview.forceActiveFocus()
+                chrome.requestedUrl = url
             }
         }
     }
@@ -475,6 +476,7 @@
             var index = tabsModel.add(tab)
             if (setCurrent) {
                 tabsModel.setCurrent(index)
+                chrome.requestedUrl = tab.initialUrl
                 if (focusAddressBar) {
                     internal.focusAddressBar()
                 }
 
=== modified file 'src/app/webbrowser/Chrome.qml'
--- src/app/webbrowser/Chrome.qml	2014-09-25 12:34:49 +0000
+++ src/app/webbrowser/Chrome.qml	2014-09-25 12:34:51 +0000
@@ -28,6 +28,7 @@
     property alias bookmarked: addressbar.bookmarked
     property list<Action> drawerActions
     readonly property bool drawerOpen: internal.openDrawer
+    property alias requestedUrl: addressbar.requestedUrl
 
     signal validated()