Fixes duplicated root domains

Resolves brave#10826 Resolves brave#9827 Auditors: Test Plan:
NejcZdovc · Sep 7, 2017 · d93dd7c · d93dd7c
1 parent 9a3ca4c
commit d93dd7c
Show file tree

Hide file tree

Showing 4 changed files with 27 additions and 5 deletions.
diff --git a/app/common/lib/siteSuggestions.js b/app/common/lib/siteSuggestions.js
@@ -7,6 +7,7 @@ const {isUrl} = require('../../../js/lib/appUrlUtil')
 const siteTags = require('../../../js/constants/siteTags')
 const urlParse = require('../urlParse')
 const Immutable = require('immutable')
+const urlUtil = require('../../../js/lib/urlutil')
 
 let take = 1000
 let initialQueTime = 50
@@ -25,7 +26,10 @@ const getSiteIdentity = (data) => {
   if (typeof data === 'string') {
     return data
   }
-  return (data.get('location') || '') + (data.get('partitionNumber') ? '|' + data.get('partitionNumber') : '')
+
+  let location = data.get('location') || ''
+  location = urlUtil.stripLocation(location)
+  return location + (data.get('partitionNumber') ? '|' + data.get('partitionNumber') : '')
 }
 
 const loadOtherSites = (sites) => {

diff --git a/app/common/lib/suggestion.js b/app/common/lib/suggestion.js
@@ -190,29 +190,39 @@ const virtualSite = (sites) => {
  * @param {object} - history
  */
 const createVirtualHistoryItems = (historySites, urlLocationLower) => {
+  let foundRoot = []
   historySites = historySites || []
 
   // parse each history item
   const parsedHistorySites = []
   historySites.forEach((site) => {
     if (site.get && site.get('location')) {
-      parsedHistorySites.push(
-        urlParse(site.get('location'))
-      )
+      const parsed = urlParse(site.get('location'))
+
+      // don't create virtual root, because we already have it
+      if (parsed.path === '/') {
+        foundRoot.push(parsed.host)
+      } else {
+        parsedHistorySites.push(parsed)
+      }
     }
   })
+
   // group them by host
   const grouped = _.groupBy(parsedHistorySites, (parsedSite) => {
     return parsedSite.host || 'unknown'
   })
+
   // find groups with more than 2 of the same host
   const multiGroupKeys = _.filter(_.keys(grouped), (k) => {
-    return grouped[k].length > 0
+    return grouped[k].length > 0 && !foundRoot.includes(k)
   })
+
   // potentially create virtual history items
   let virtualHistorySites = _.map(multiGroupKeys, (location) => {
     return virtualSite(grouped[location])
   })
+
   virtualHistorySites = _.filter(virtualHistorySites, (site) => {
     return !!site
   })

diff --git a/js/lib/urlutil.js b/js/lib/urlutil.js
@@ -441,6 +441,13 @@ const UrlUtil = {
       return parsed.slashes ? [parsed.protocol, parsed.host].join('//') : [parsed.protocol, parsed.host].join('')
     }
     return null
+  },
+
+  stripLocation: (url) => {
+    return url
+      .replace(/\/*$/, '')  // remove trailing /
+      .replace(/#*$/, '') // remove trailing  #
+      .trim() // remove whitespaces
   }
 }
 

diff --git a/test/navbar-components/urlBarTest.js b/test/navbar-components/urlBarTest.js
@@ -269,6 +269,7 @@ describe('urlBar tests', function () {
           .tabByIndex(0)
           .loadUrl(this.page)
           .windowByUrl(Brave.browserWindowUrl)
+          .activateURLMode()
           .ipcSend('shortcut-focus-url')
           .waitForElementFocus(urlInput)
           .setValue(urlInput, 'google')