Merge lp:~daker/webbrowser-app/fix.1166063 into lp:webbrowser-app

Proposed by Adnane Belmadiaf
Status: Merged
Approved by: Olivier Tilloy
Approved revision: 304
Merged at revision: 308
Proposed branch: lp:~daker/webbrowser-app/fix.1166063
Merge into: lp:webbrowser-app
Diff against target: 36 lines (+15/-0)
2 files modified
src/app/AddressBar.qml (+3/-0)
tests/unittests/qml/tst_AddressBar.qml (+12/-0)
To merge this branch: bzr merge lp:~daker/webbrowser-app/fix.1166063
Reviewer Review Type Date Requested Status
PS Jenkins bot continuous-integration Approve
Olivier Tilloy Approve
Review via email: mp+185391@code.launchpad.net

Commit message

Added support for IP adresses

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

Thanks for tackling this bug!

A couple of remarks:

 - I think using a regular expression to match an IP address would be more efficient. A simple regexp would be enough (no need to validate that the four components are in the range 0-255, if they are not it’s very likely a typo and the user will realize it and correct it).

 - Can you add a unit test for IP addresses in tests/unittests/qml/tst_AddressBar.qml ?

Revision history for this message
Adnane Belmadiaf (daker) wrote :

Sure i'll fix it

lp:~daker/webbrowser-app/fix.1166063 updated
304. By Adnane Belmadiaf

Fixed regexp for ip addresses
Added unit test for the ip addresses

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

Looks perfect, thanks!

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

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'src/app/AddressBar.qml'
--- src/app/AddressBar.qml 2013-08-08 16:32:40 +0000
+++ src/app/AddressBar.qml 2013-09-15 17:17:42 +0000
@@ -134,6 +134,9 @@
134 if (address.split('/', 1)[0].match(/\.[a-z]{2,4}$/)) {134 if (address.split('/', 1)[0].match(/\.[a-z]{2,4}$/)) {
135 return true135 return true
136 }136 }
137 if (address.split('/', 1)[0].match(/^(?:[0-9]{1,3}\.){3}[0-9]{1,3}/)) {
138 return true
139 }
137 return false140 return false
138 }141 }
139142
140143
=== modified file 'tests/unittests/qml/tst_AddressBar.qml'
--- tests/unittests/qml/tst_AddressBar.qml 2013-07-18 12:04:50 +0000
+++ tests/unittests/qml/tst_AddressBar.qml 2013-09-15 17:17:42 +0000
@@ -47,6 +47,18 @@
47 compare(addressBar.requestedUrl, "http://ubuntu.com")47 compare(addressBar.requestedUrl, "http://ubuntu.com")
48 }48 }
4949
50 function test_no_ipadress_scheme_rewrite() {
51 addressBar.text = "192.168.1.1"
52 addressBar.validate()
53 compare(addressBar.requestedUrl, "http://192.168.1.1")
54 addressBar.text = "192.168.1.1:8000"
55 addressBar.validate()
56 compare(addressBar.requestedUrl, "http://192.168.1.1:8000")
57 addressBar.text = "192.168.1.1:8000/dummy.html"
58 addressBar.validate()
59 compare(addressBar.requestedUrl, "http://192.168.1.1:8000/dummy.html")
60 }
61
50 function test_unhandled_scheme_no_rewrite() {62 function test_unhandled_scheme_no_rewrite() {
51 addressBar.text = "ftp://ubuntu.com"63 addressBar.text = "ftp://ubuntu.com"
52 addressBar.validate()64 addressBar.validate()

Subscribers

People subscribed via source and target branches

to status/vote changes: