Improve sorting for matching subdomains

Auditors: @bsclifton

Fix #8982

Author: bbondy

app/common/lib/suggestion.js

@@ -234,8 +234,10 @@ const getSortByDomain = (userInputLower, userInputHost) => {
     // any count or frequency calculation.
     // Note that for parsed URLs that are not complete, the pathname contains
     // what the user is entering as the host and the host is null.
-    const host1 = s1.parsedUrl.host || s1.parsedUrl.pathname || s1.location || ''
-    const host2 = s2.parsedUrl.host || s2.parsedUrl.pathname || s2.location || ''
+    let host1 = s1.parsedUrl.host || s1.parsedUrl.pathname || s1.location || ''
+    let host2 = s2.parsedUrl.host || s2.parsedUrl.pathname || s2.location || ''
+    host1 = host1.replace('www.', '')
+    host2 = host2.replace('www.', '')
 
     let pos1 = host1.indexOf(userInputHost)
     let pos2 = host2.indexOf(userInputHost)
@@ -255,18 +257,6 @@ const getSortByDomain = (userInputLower, userInputHost) => {
         return 2
       }
 
-      // Try the same to see if taking off www. helps.
-      if (!userInputLower.startsWith('www.')) {
-        pos1 = host1.indexOf('www.' + userInputLower)
-        pos2 = host2.indexOf('www.' + userInputLower)
-        if (pos1 === 0 && pos2 !== 0) {
-          return -1
-        }
-        if (pos1 !== 0 && pos2 === 0) {
-          return 1
-        }
-      }
-
       const sortBySimpleURLResult = sortBySimpleURL(s1, s2)
       if (sortBySimpleURLResult !== 0) {
         return sortBySimpleURLResult
@@ -300,6 +290,25 @@ const sortBySimpleURL = (s1, s2) => {
     if (!url1IsSecure && url2IsSecure) {
       return 1
     }
+
+    // Prefer smaller less complicated domains
+    const parts1 = s1.parsedUrl.hostname.split('.')
+    const parts2 = s2.parsedUrl.hostname.split('.')
+    let parts1Size = parts1.length
+    let parts2Size = parts2.length
+    if (parts1[0] === 'www') {
+      parts1Size--
+    }
+    if (parts2[0] === 'www') {
+      parts2Size--
+    }
+    if (parts1Size < parts2Size) {
+      return -1
+    }
+    if (parts1Size > parts2Size) {
+      return 1
+    }
+    return sortByAccessCountWithAgeDecay(s1, s2)
   }
   return 0
 }
@@ -333,7 +342,6 @@ const getSortForSuggestions = (userInputLower) => {
   const userInputValue = userInputParts[1] || ''
   const sortByDomain = getSortByDomain(userInputLower, userInputHost)
   const sortByPath = getSortByPath(userInputLower)
-  const {sortByAccessCountWithAgeDecay} = require('./suggestion')
 
   return (s1, s2) => {
     s1.parsedUrl = s1.parsedUrl || urlParse(getURL(s1) || '')

test/unit/app/common/lib/suggestionTest.js

@@ -212,6 +212,12 @@ describe('suggestion unit tests', function () {
       it('Prefers https sipmle URLs', function () {
         assert(this.sort('https://brave.com', 'http://brave.com') < 0)
       })
+      it('sorts better matched domains based on more simple domains', function () {
+        assert(this.sort('https://facebook.github.io/', 'https://facebook.com/') > 0)
+      })
+      it('sorts better matched domains based on more simple domains ignoring www.', function () {
+        assert(this.sort('https://facebook.github.io/', 'https://www.facebook.com/') > 0)
+      })
     })
     describe('getSortByDomain', function () {
       before(function () {
@@ -241,9 +247,6 @@ describe('suggestion unit tests', function () {
       it('negative if first site has a match from the start of domain', function () {
         assert(this.sort('https://google.com', 'https://mygoogle.com') < 0)
       })
-      it('positive if second site has a match but without www.', function () {
-        assert(this.sort('https://www.google.com', 'https://google.com') > 0)
-      })
       it('negative if there is a pos 0 match not including www.', function () {
         assert(this.sort('https://www.google.com', 'https://mygoogle.com') < 0)
       })
@@ -253,6 +256,34 @@ describe('suggestion unit tests', function () {
       it('does not throw error for file:// URL', function () {
         assert(this.sort('https://google.com', 'file://') < 0)
       })
+      it('sorts simple domains that match equally on subdomains as the same', function () {
+        const url1 = 'https://facebook.github.com'
+        const url2 = 'https://facebook.brave.com'
+        const sort = suggestion.getSortByDomain('facebook', 'facebook')
+        assert(sort({
+          location: url1,
+          parsedUrl: urlParse(url1)
+        }, {
+          location: url2,
+          parsedUrl: urlParse(url2)
+        }) === 0)
+      })
+      it('sorts simple domains that match equally but have different activity based on activity', function () {
+        const url1 = 'https://facebook.github.com'
+        const url2 = 'https://facebook.brave.com'
+        const sort = suggestion.getSortByDomain('facebook', 'facebook')
+        assert(sort({
+          location: url1,
+          parsedUrl: urlParse(url1),
+          lastAccessedTime: 1495335766455,
+          count: 30
+        }, {
+          location: url2,
+          parsedUrl: urlParse(url2),
+          lastAccessedTime: 1495334766432,
+          count: 10
+        }) < 0)
+      })
     })
     describe('getSortForSuggestions', function () {
       describe('with url entered as path', function () {
@@ -299,6 +330,17 @@ describe('suggestion unit tests', function () {
           assert(this.sort('https://brianbondy.com', 'www.brianbondy.com') < 0)
         })
       })
+      it('sorts better matched domains based on more simple domains ignoring www.', function () {
+        const userInputLower = 'facebook'
+        const internalSort = suggestion.getSortForSuggestions(userInputLower, userInputLower)
+        const sort = (url1, url2) => {
+          return internalSort(
+            { location: url1, parsedUrl: urlParse(url1) },
+            { location: url2, parsedUrl: urlParse(url2) }
+          )
+        }
+        assert(sort('https://facebook.github.io/', 'https://www.facebook.com/') > 0)
+      })
     })
   })
 })