From fd20405ddf27365cfaf69e16289b2fc6d39c2a5e Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Wed, 14 Sep 2016 06:10:08 +0100 Subject: Match globa-mark URLs by length. This changes the logic for selecting an existing tab when using global marks. Previously, an exact RUL match was required. Here, we only require a prefix match. For example, if the global-mark URL is: https://inbox.google.com/u/0/ Then a tab with the URL https://inbox.google.com/u/0/sent will be selected. This is a more usable approach when the user uses global marks to visit important sites like gmail, Facebook or Twitter. On these sites, the URL changes, but the user still thinks of the tab as their "gmail tab", for example. Also, when choosing between multiple candidate tabs: - If there is at least one candidate in the current window, then only consider candidates in the current window. - If there are more than one candidates, then don't select the current tab. - Finally, select the remaining candidate with the shortest URL. Closes #2248. --- background_scripts/marks.coffee | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/background_scripts/marks.coffee b/background_scripts/marks.coffee index 8fe30b2f..29029b4d 100644 --- a/background_scripts/marks.coffee +++ b/background_scripts/marks.coffee @@ -63,10 +63,10 @@ Marks = # The tab we're trying to find no longer exists. We either find another tab with a matching URL and use it, # or we create a new tab. focusOrLaunch: (markInfo, req) -> - chrome.tabs.query { url: markInfo.url }, (tabs) => + chrome.tabs.query { url: "#{markInfo.url}*" }, (tabs) => if 0 < tabs.length - # We have a matching tab: use it (prefering, if there are more than one, one in the current window). - @pickTabInWindow tabs, (tab) => + # We have at least one matching tab. Pick one and go to it. + @pickTab tabs, (tab) => @gotoPositionInTab extend markInfo, tabId: tab.id else # There is no existing matching tab, we'll have to create one. @@ -75,11 +75,18 @@ Marks = # is loaded, its DOM is ready and it registers with the background page. tabLoadedHandlers[tab.id] = => @gotoPositionInTab extend markInfo, tabId: tab.id - # Given a list of tabs, pick one in the current window, if possible, otherwise just pick any. - pickTabInWindow: (tabs, continuation) -> + # Given a list of tabs candidate tabs, pick one. Prefer tabs in the current window and tabs with shorter + # (matching) URLs. + pickTab: (tabs, callback) -> chrome.windows.getCurrent ({ id }) -> + # Prefer tabs in the current window, if there are any. tabsInWindow = tabs.filter (tab) -> tab.windowId == id - continuation tabsInWindow[0] ? tabs[0] + tabs = tabsInWindow if 0 < tabsInWindow.length + # If more than one tab remains and the current tab remains a conadidate, then pick another one. + tabs = (tab for tab in tabs when not tab.active) if 1 < tabs.length + # Prefer shorter URLs. + tabs.sort (a,b) -> a.url.length - b.url.length + callback tabs[0] root = exports ? window root.Marks = Marks -- cgit v1.2.3 From 722cd3c40f89d93682f08e88b6d4cb46ca70991e Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Wed, 14 Sep 2016 08:02:22 +0100 Subject: Require exact match when we will be scrolling. If the user is jumping to a scroll position within a tab, then we need an exact match on the URL (because otherwise the scroll position doesn't really have a meaning). Otherwise we only require a prefix match, a la #2250. --- background_scripts/marks.coffee | 3 +++ content_scripts/vimium_frontend.coffee | 4 ++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/background_scripts/marks.coffee b/background_scripts/marks.coffee index 29029b4d..daf28748 100644 --- a/background_scripts/marks.coffee +++ b/background_scripts/marks.coffee @@ -63,6 +63,9 @@ Marks = # The tab we're trying to find no longer exists. We either find another tab with a matching URL and use it, # or we create a new tab. focusOrLaunch: (markInfo, req) -> + # If we're not going to be scrolling to a particular position in the tab, then we choose all tabs with a + # matching URL prefix. Otherwise, we require an exact match. + query = if markInfo.scrollX == markInfo.scrollY == 0 then "#{markInfo.url}*" else markInfo.url chrome.tabs.query { url: "#{markInfo.url}*" }, (tabs) => if 0 < tabs.length # We have at least one matching tab. Pick one and go to it. diff --git a/content_scripts/vimium_frontend.coffee b/content_scripts/vimium_frontend.coffee index f11c8f4b..aea1b6ac 100644 --- a/content_scripts/vimium_frontend.coffee +++ b/content_scripts/vimium_frontend.coffee @@ -257,8 +257,8 @@ Frame = window.removeEventListener "hashchange", onFocus setScrollPosition = ({ scrollX, scrollY }) -> - if DomUtils.isTopFrame() - DomUtils.documentReady -> + DomUtils.documentReady -> + if DomUtils.isTopFrame() window.focus() document.body.focus() if 0 < scrollX or 0 < scrollY -- cgit v1.2.3 From 48e3893076a6ca93223743bee73b713721070eb5 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Wed, 14 Sep 2016 08:10:55 +0100 Subject: Fix omission from 722cd3c40f89d93682f08e88b6d4cb46ca70991e. --- background_scripts/marks.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/background_scripts/marks.coffee b/background_scripts/marks.coffee index daf28748..963b8925 100644 --- a/background_scripts/marks.coffee +++ b/background_scripts/marks.coffee @@ -66,7 +66,7 @@ Marks = # If we're not going to be scrolling to a particular position in the tab, then we choose all tabs with a # matching URL prefix. Otherwise, we require an exact match. query = if markInfo.scrollX == markInfo.scrollY == 0 then "#{markInfo.url}*" else markInfo.url - chrome.tabs.query { url: "#{markInfo.url}*" }, (tabs) => + chrome.tabs.query { url: query }, (tabs) => if 0 < tabs.length # We have at least one matching tab. Pick one and go to it. @pickTab tabs, (tab) => -- cgit v1.2.3 From d6b3845a8a9d7604bad7f1a24d87c39bdc116125 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Sat, 17 Sep 2016 08:03:25 +0100 Subject: Tweaks to comments. --- background_scripts/marks.coffee | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/background_scripts/marks.coffee b/background_scripts/marks.coffee index 963b8925..dbc14671 100644 --- a/background_scripts/marks.coffee +++ b/background_scripts/marks.coffee @@ -64,7 +64,8 @@ Marks = # or we create a new tab. focusOrLaunch: (markInfo, req) -> # If we're not going to be scrolling to a particular position in the tab, then we choose all tabs with a - # matching URL prefix. Otherwise, we require an exact match. + # matching URL prefix. Otherwise, we require an exact match (because it doesn't make sense to scroll + # unless there's an exact URL match). query = if markInfo.scrollX == markInfo.scrollY == 0 then "#{markInfo.url}*" else markInfo.url chrome.tabs.query { url: query }, (tabs) => if 0 < tabs.length @@ -85,7 +86,8 @@ Marks = # Prefer tabs in the current window, if there are any. tabsInWindow = tabs.filter (tab) -> tab.windowId == id tabs = tabsInWindow if 0 < tabsInWindow.length - # If more than one tab remains and the current tab remains a conadidate, then pick another one. + # If more than one tab remains and the current tab is still a candidate, then don't pick the current + # tab (because jumping to it does nothing). tabs = (tab for tab in tabs when not tab.active) if 1 < tabs.length # Prefer shorter URLs. tabs.sort (a,b) -> a.url.length - b.url.length -- cgit v1.2.3