Merge lp:~rpadovani/webbrowser-app/data-address into lp:webbrowser-app

[Riccardo Padovani]

Add support for data: URIs in the address bar, and remove length limitation for TLDs.

Fix bug #1377953 - data: URIs don't work

You can find some tests here: http://www-archive.mozilla.org/quality/networking/testing/datatests.html

They don't work all as expected (as described in the page), but they work as Chromium on vivid, so I think it's acceptable

Fix bug #1441281 - update looksLikeAUrl().

Remove .tdl regex length limitation (min 2, max *)

Based on lp:~osomon/webbrowser-app/about-blank

[lp:~rpadovani/webbrowser-app/data-address]

969. By Riccardo Padovani on 2015-04-15

Update tests

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:969
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/1650/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/2262/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-amd64-ci/407/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/407/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-i386-ci/407/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/2260/console

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

review: Needs Fixing (continuous-integration)

[Olivier Tilloy (osomon)]

8 + } else if (address.substr(0, 4).toLowerCase == "data:") {
9 + return address.substr(0, 4).toLowerCase + address.substr(5);

I’d suggest using a regexp instead of String.substr(…), and no need to do the substr() dance again if you know that the scheme in lowercase is "data":

    if (address.match(/^data:/i)) {
        return "data:" + address.substr(5)
    }

Note that the CI job failed because your changes break the existing unit tests, probably because the call to substr should have been with a length of 5, not 4.

review: Needs Fixing

[lp:~rpadovani/webbrowser-app/data-address]

970. By Riccardo Padovani on 2015-04-16

Use regex instead of substr

[Riccardo Padovani (rpadovani)]

Thanks for the feedback, you're totally right :-)

Fixed!

[Olivier Tilloy (osomon)]

I’m still seeing unit test failures. To run the unit tests locally and fix them:

    ctest -V -R tst_QmlTests

review: Needs Fixing

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:970
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/1660/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/2290/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-amd64-ci/417/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/417/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-i386-ci/417/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/2288/console

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

review: Needs Fixing (continuous-integration)

[lp:~rpadovani/webbrowser-app/data-address]

971. By Riccardo Padovani on 2015-04-27

Fix data uri tests

972. By Riccardo Padovani on 2015-04-27

Merge from upstream

973. By Riccardo Padovani on 2015-04-27

Fix implementation of urlManagement for data: address

[Riccardo Padovani (rpadovani)]

Tests fail because my implementation was wrong :-)

So, the data: address could has space in the url, so I need to check if is a data: address before checking if there are space in the address.

Then, for tdl, I need to check if the tld is > 2 ({2,}) and not 2 ({2}). Now should be all ok!

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:973
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/1690/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/2449/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-amd64-ci/447
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/447
        deb: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/447/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-i386-ci/447
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-vivid-mako/2128/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/2447
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/2447/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/19923

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

review: Needs Fixing (continuous-integration)

[Olivier Tilloy (osomon)]

There’s no new unit test for longer TLDs.
The changes otherwise look good.

review: Needs Fixing

[lp:~rpadovani/webbrowser-app/data-address]

974. By Riccardo Padovani on 2015-04-28

Add test for tld longer than 3 chars

[Riccardo Padovani (rpadovani)]

Right.

I used http://com.google because it's the only valid one I know, is it ok?

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:974
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/1697/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/2478/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-amd64-ci/454
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/454
        deb: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/454/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-i386-ci/454
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-vivid-mako/2150/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/2476
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/2476/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/19962

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

review: Needs Fixing (continuous-integration)

[Olivier Tilloy (osomon)]

That looks good now, thanks!

review: Approve

=== modified file 'src/app/webbrowser/AddressBar.qml'
--- src/app/webbrowser/AddressBar.qml	2015-04-14 21:13:19 +0000
+++ src/app/webbrowser/AddressBar.qml	2015-04-28 07:20:23 +0000
@@ -257,7 +257,7 @@
 
         function simplifyUrl(url) {
             var urlString = url.toString()
-            if (urlString == "about:blank") {
+            if (urlString == "about:blank" || urlString.match(/^data:/i)) {
                 return url
             }
             var hasProtocol = urlString.indexOf("://") != -1

=== modified file 'src/app/webbrowser/urlManagement.js'
--- src/app/webbrowser/urlManagement.js	2015-04-14 21:13:19 +0000
+++ src/app/webbrowser/urlManagement.js	2015-04-28 07:20:23 +0000
@@ -21,6 +21,8 @@
     var url = address
     if (address.toLowerCase() == "about:blank") {
         return address.toLowerCase()
+    } else if (address.match(/^data:/i)) {
+        return "data:" + address.substr(5)
     } else if (address.substr(0, 1) == "/") {
         url = "file://" + address
     } else if (address.indexOf("://") == -1) {
@@ -30,6 +32,9 @@
 }
 
 function looksLikeAUrl(address) {
+    if (address.match(/^data:/i)) {
+        return true;
+    }
     var terms = address.split(/\s/)
     if (terms.length > 1) {
         return false
@@ -45,7 +50,7 @@
         address.match(/^[a-z]+:\/\//i)) {
         return true
     }
-    if (address.split('/', 1)[0].match(/\.[a-zA-Z]{2,4}$/)) {
+    if (address.split('/', 1)[0].match(/\.[a-zA-Z]{2,}$/)) {
         return true
     }
     if (address.split('/', 1)[0].match(/^(?:[0-9]{1,3}\.){3}[0-9]{1,3}/)) {

=== modified file 'tests/unittests/qml/tst_AddressBar.qml'
--- tests/unittests/qml/tst_AddressBar.qml	2015-04-14 21:13:19 +0000
+++ tests/unittests/qml/tst_AddressBar.qml	2015-04-28 07:20:23 +0000
@@ -91,6 +91,8 @@
                 {url: "https://google.com"},
                 {url: "ftp://ubuntu.com"},
                 {url: "about:blank"},
+                {url: "data:,A brief note"},
+                {url: "http://com.google"}
             ]
         }
 
@@ -194,6 +196,8 @@
                 {text: "FILE:///usr/share/doc/ubuntu-online-tour/index.html", requestedUrl: "file:///usr/share/doc/ubuntu-online-tour/index.html"},
                 {text: "FTP://ubuntu.com", requestedUrl: "ftp://ubuntu.com"},
                 {text: "ABOUT:BLANK", requestedUrl: "about:blank"},
+                {text: "DATA:,A brief note", requestedUrl: "data:,A brief note"},
+                {text: "HTTP://com.GOOGLE", requestedUrl: "http://com.google"}
             ]
         }
 
@@ -242,7 +246,10 @@
                  actualUrl: "http://en.wikipedia.org"},
                 {input: "en.wikipedia.org/wiki/Foo",
                  simplified: "en.wikipedia.org",
-                 actualUrl: "http://en.wikipedia.org/wiki/Foo"}
+                 actualUrl: "http://en.wikipedia.org/wiki/Foo"},
+                {input: "http://com.google",
+                 simplified: "com.google",
+                 actualUrl: "http://com.google"},
             ]
         }