Merge branch 'smblott-github-domains-only-domains'

4 files changed, 28 insertions, 18 deletions

diff --git a/background_scripts/completion.coffee b/background_scripts/completion.coffee
index 23696185..b75ebb87 100644
--- a/background_scripts/completion.coffee
+++ b/background_scripts/completion.coffee
@@ -238,7 +238,7 @@ class DomainCompleter
       onComplete()
 
   onPageVisited: (newPage) ->
-    domain = @parseDomain(newPage.url)
+    domain = @parseDomainAndScheme newPage.url
     if domain
       slot = @domains[domain] ||= { entry: newPage, referenceCount: 0 }
       # We want each entry in our domains hash to point to the most recent History entry for that domain.
@@ -250,11 +250,13 @@ class DomainCompleter
       @domains = {}
     else
       toRemove.urls.forEach (url) =>
-        domain = @parseDomain(url)
+        domain = @parseDomainAndScheme url
         if domain and @domains[domain] and ( @domains[domain].referenceCount -= 1 ) == 0
           delete @domains[domain]
 
-  parseDomain: (url) -> url.split("/")[2] || ""
+  # Return something like "http://www.example.com" or false.
+  parseDomainAndScheme: (url) ->
+      Utils.hasFullUrlPrefix(url) and not Utils.hasChromePrefix(url) and url.split("/",3).join "/"
 
   # Suggestions from the Domain completer have the maximum relevancy. They should be shown first in the list.
   computeRelevancy: -> 1
diff --git a/lib/utils.coffee b/lib/utils.coffee
index bbcee1a0..b7f8731a 100644
--- a/lib/utils.coffee
+++ b/lib/utils.coffee
@@ -26,28 +26,29 @@ Utils =
     -> id += 1
 
   hasChromePrefix: do ->
-    chromePrefixes = [ "about:", "view-source:", "chrome-extension:", "data:" ]
+    chromePrefixes = [ "about:", "view-source:", "extension:", "chrome-extension:", "data:" ]
     (url) ->
       if 0 < url.indexOf ":"
         for prefix in chromePrefixes
           return true if url.startsWith prefix
       false
 
+  hasFullUrlPrefix: do ->
+    urlPrefix = new RegExp "^[a-z]{3,}://."
+    (url) -> urlPrefix.test url
+
   # Completes a partial URL (without scheme)
   createFullUrl: (partialUrl) ->
-    unless /^[a-z]{3,}:\/\//.test partialUrl
-      "http://" + partialUrl
-    else
-      partialUrl
+    if @hasFullUrlPrefix(partialUrl) then partialUrl else ("http://" + partialUrl)
 
   # Tries to detect if :str is a valid URL.
   isUrl: (str) ->
-    # Starts with a scheme: URL
-    return true if /^[a-z]{3,}:\/\//.test str
-
     # Must not contain spaces
     return false if ' ' in str
 
+    # Starts with a scheme: URL
+    return true if @hasFullUrlPrefix str
+
     # More or less RFC compliant URL host part parsing. This should be sufficient for our needs
     urlRegex = new RegExp(
       '^(?:([^:]+)(?::([^:]+))?@)?' + # user:password (optional) => \1, \2
@@ -98,7 +99,7 @@ Utils =
   convertToUrl: (string) ->
     string = string.trim()
 
-    # Special-case about:[url] and view-source:[url]
+    # Special-case about:[url], view-source:[url] and the like
     if Utils.hasChromePrefix string
       string
     else if Utils.isUrl string
diff --git a/tests/unit_tests/completion_test.coffee b/tests/unit_tests/completion_test.coffee
index 811436a9..44989267 100644
--- a/tests/unit_tests/completion_test.coffee
+++ b/tests/unit_tests/completion_test.coffee
@@ -163,13 +163,13 @@ context "domain completer",
 
   should "return only a single matching domain", ->
     results = filterCompleter(@completer, ["story"])
-    assert.arrayEqual ["history1.com"], results.map (result) -> result.url
+    assert.arrayEqual ["http://history1.com"], results.map (result) -> result.url
 
   should "pick domains which are more recent", ->
     # These domains are the same except for their last visited time.
-    assert.equal "history1.com", filterCompleter(@completer, ["story"])[0].url
+    assert.equal "http://history1.com", filterCompleter(@completer, ["story"])[0].url
     @history2.lastVisitTime = hours(3)
-    assert.equal "history2.com", filterCompleter(@completer, ["story"])[0].url
+    assert.equal "http://history2.com", filterCompleter(@completer, ["story"])[0].url
 
   should "returns no results when there's more than one query term, because clearly it's not a domain", ->
     assert.arrayEqual [], filterCompleter(@completer, ["his", "tory"])
@@ -194,15 +194,15 @@ context "domain completer (removing entries)",
 
   should "remove 1 entry for domain with reference count of 1", ->
     @onVisitRemovedListener { allHistory: false, urls: [@history1.url] }
-    assert.equal "history2.com", filterCompleter(@completer, ["story"])[0].url
+    assert.equal "http://history2.com", filterCompleter(@completer, ["story"])[0].url
     assert.equal 0, filterCompleter(@completer, ["story1"]).length
 
   should "remove 2 entries for domain with reference count of 2", ->
     @onVisitRemovedListener { allHistory: false, urls: [@history2.url] }
-    assert.equal "history2.com", filterCompleter(@completer, ["story2"])[0].url
+    assert.equal "http://history2.com", filterCompleter(@completer, ["story2"])[0].url
     @onVisitRemovedListener { allHistory: false, urls: [@history3.url] }
     assert.equal 0, filterCompleter(@completer, ["story2"]).length
-    assert.equal "history1.com", filterCompleter(@completer, ["story"])[0].url
+    assert.equal "http://history1.com", filterCompleter(@completer, ["story"])[0].url
 
   should "remove 3 (all) matching domain entries", ->
     @onVisitRemovedListener { allHistory: false, urls: [@history2.url] }
diff --git a/tests/unit_tests/utils_test.coffee b/tests/unit_tests/utils_test.coffee
index b2d656ab..556f5b7a 100644
--- a/tests/unit_tests/utils_test.coffee
+++ b/tests/unit_tests/utils_test.coffee
@@ -61,6 +61,13 @@ context "hasChromePrefix",
     assert.isFalse Utils.hasChromePrefix "data"
     assert.isFalse Utils.hasChromePrefix "data :foobar"
 
+context "isUrl",
+  should "identify URLs as URLs", ->
+    assert.isTrue Utils.isUrl "http://www.example.com/blah"
+
+  should "identify non-URLs and non-URLs", ->
+    assert.isFalse Utils.isUrl "http://www.example.com/ blah"
+
 context "Function currying",
   should "Curry correctly", ->
     foo = (a, b) -> "#{a},#{b}"