From c08d03e36e3c6a08fe9eaa493fb2a3262f77b74b Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Thu, 6 Nov 2014 06:30:48 +0000 Subject: Don't suggest domains which aren't. --- background_scripts/completion.coffee | 2 +- lib/utils.coffee | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/background_scripts/completion.coffee b/background_scripts/completion.coffee index 23696185..0a936459 100644 --- a/background_scripts/completion.coffee +++ b/background_scripts/completion.coffee @@ -239,7 +239,7 @@ class DomainCompleter onPageVisited: (newPage) -> domain = @parseDomain(newPage.url) - if domain + if domain and not Utils.hasChromePrefix newPage.url 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. slot.entry = newPage if slot.entry.lastVisitTime < newPage.lastVisitTime diff --git a/lib/utils.coffee b/lib/utils.coffee index bbcee1a0..e2ed8d98 100644 --- a/lib/utils.coffee +++ b/lib/utils.coffee @@ -26,7 +26,7 @@ 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 @@ -98,7 +98,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 -- cgit v1.2.3 From 46ee1444e08815cd3ec4d1f008d62d13b35ad777 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Thu, 6 Nov 2014 08:50:59 +0000 Subject: Include scheme for domain suggestions; tighter test for valid domain. --- background_scripts/completion.coffee | 10 ++++++---- lib/utils.coffee | 11 ++++++----- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/background_scripts/completion.coffee b/background_scripts/completion.coffee index 0a936459..b75ebb87 100644 --- a/background_scripts/completion.coffee +++ b/background_scripts/completion.coffee @@ -238,8 +238,8 @@ class DomainCompleter onComplete() onPageVisited: (newPage) -> - domain = @parseDomain(newPage.url) - if domain and not Utils.hasChromePrefix 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. slot.entry = newPage if slot.entry.lastVisitTime < newPage.lastVisitTime @@ -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 e2ed8d98..62749496 100644 --- a/lib/utils.coffee +++ b/lib/utils.coffee @@ -33,17 +33,18 @@ Utils = 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 + return true if @hasFullUrlPrefix str # Must not contain spaces return false if ' ' in str -- cgit v1.2.3 From 7f4eb88ea2d79e670c4383120fb5e1ac139e2778 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Thu, 6 Nov 2014 08:53:57 +0000 Subject: Fix domain tests. --- tests/unit_tests/completion_test.coffee | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) 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] } -- cgit v1.2.3 From bbea4b123fe9da0511b1698f07868c27299ae24f Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Thu, 6 Nov 2014 09:01:22 +0000 Subject: Add (initial, basic) isUrl tests (more needed). --- lib/utils.coffee | 6 +++--- tests/unit_tests/utils_test.coffee | 7 +++++++ 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/lib/utils.coffee b/lib/utils.coffee index 62749496..b7f8731a 100644 --- a/lib/utils.coffee +++ b/lib/utils.coffee @@ -43,12 +43,12 @@ Utils = # Tries to detect if :str is a valid URL. isUrl: (str) -> - # Starts with a scheme: URL - return true if @hasFullUrlPrefix 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 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}" -- cgit v1.2.3