From c567d7d043b689d72eabbd671eab8ae8805dadc1 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Wed, 3 Jun 2015 11:23:29 +0100 Subject: Fix marks (incl. global marks)... Fixes #1712: - Make global marks work. - Add mode indicator. - Don't fail for global marks on background page if mark is not set. - Give HUD warning for global marks if global mark is not set. (The diff is big but, which the exception of infrastructure refactoring, the main change is to not exit on , thereby fixing #1712). --- content_scripts/marks.coffee | 101 +++++++++++++++++++++++++------------------ 1 file changed, 60 insertions(+), 41 deletions(-) (limited to 'content_scripts') diff --git a/content_scripts/marks.coffee b/content_scripts/marks.coffee index 316ab951..f3dfd465 100644 --- a/content_scripts/marks.coffee +++ b/content_scripts/marks.coffee @@ -1,45 +1,64 @@ -root = window.Marks = {} -root.activateCreateMode = -> - handlerStack.push keydown: (e) -> - keyChar = KeyboardUtils.getKeyChar(event) - return unless keyChar isnt "" +exit = (mode, continuation = null) -> + mode.exit() + continuation?() + false - if /[A-Z]/.test keyChar - chrome.runtime.sendMessage { - handler: 'createMark', - markName: keyChar - scrollX: window.scrollX, - scrollY: window.scrollY - }, -> HUD.showForDuration "Created global mark '#{keyChar}'", 1000 - else if /[a-z]/.test keyChar - [baseLocation, sep, hash] = window.location.href.split '#' - localStorage["vimiumMark|#{baseLocation}|#{keyChar}"] = JSON.stringify - scrollX: window.scrollX, - scrollY: window.scrollY - HUD.showForDuration "Created local mark '#{keyChar}'", 1000 +Marks = + activateCreateMode: -> + mode = new Mode + name: "create-mark" + indicator: "Create mark?" + keypress: -> false + keyup: -> false + keydown: (event) -> + keyChar = KeyboardUtils.getKeyChar(event) + if /[A-Z]/.test keyChar + exit mode, -> + chrome.runtime.sendMessage + handler: 'createMark' + markName: keyChar + scrollX: window.scrollX + scrollY: window.scrollY + , -> HUD.showForDuration "Created global mark '#{keyChar}'.", 1000 + else if /[a-z]/.test keyChar + [baseLocation, sep, hash] = window.location.href.split '#' + localStorage["vimiumMark|#{baseLocation}|#{keyChar}"] = JSON.stringify + scrollX: window.scrollX, + scrollY: window.scrollY + exit mode, -> HUD.showForDuration "Created local mark '#{keyChar}'.", 1000 + else if event.shiftKey + false + else + exit mode - @remove() + activateGotoMode: -> + mode = new Mode + name: "goto-mark" + indicator: "Go to mark?" + keypress: -> false + keyup: -> false + keydown: (event) -> + keyChar = KeyboardUtils.getKeyChar(event) + if /[A-Z]/.test keyChar + exit mode, -> + chrome.runtime.sendMessage + handler: 'gotoMark' + markName: keyChar + else if /[a-z]/.test keyChar + [baseLocation, sep, hash] = window.location.href.split '#' + markString = localStorage["vimiumMark|#{baseLocation}|#{keyChar}"] + exit mode, -> + if markString? + mark = JSON.parse markString + window.scrollTo mark.scrollX, mark.scrollY + HUD.showForDuration "Jumped to local mark '#{keyChar}'", 1000 + else + HUD.showForDuration "Local mark not set: '#{keyChar}'.", 1000 + else if event.shiftKey + false + else + exit mode - false - -root.activateGotoMode = -> - handlerStack.push keydown: (e) -> - keyChar = KeyboardUtils.getKeyChar(event) - return unless keyChar isnt "" - - if /[A-Z]/.test keyChar - chrome.runtime.sendMessage - handler: 'gotoMark' - markName: keyChar - else if /[a-z]/.test keyChar - [baseLocation, sep, hash] = window.location.href.split '#' - markString = localStorage["vimiumMark|#{baseLocation}|#{keyChar}"] - if markString? - mark = JSON.parse markString - window.scrollTo mark.scrollX, mark.scrollY - HUD.showForDuration "Jumped to local mark '#{keyChar}'", 1000 - - @remove() - - false +root = exports ? window +root.Marks = Marks -- cgit v1.2.3 From 03ef7f2a8525e8fbfc67b04d7a0ce47522449d03 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Wed, 3 Jun 2015 11:42:25 +0100 Subject: Refactor to avoid having to cover all keyboard event cases. It's pretty common that we want to suppress all keyboard events, so let's support that in modes.coffee, thereby simplifying handlers elsewhere. --- content_scripts/marks.coffee | 15 ++++----------- content_scripts/mode.coffee | 9 +++++++++ 2 files changed, 13 insertions(+), 11 deletions(-) (limited to 'content_scripts') diff --git a/content_scripts/marks.coffee b/content_scripts/marks.coffee index f3dfd465..8ba45fd4 100644 --- a/content_scripts/marks.coffee +++ b/content_scripts/marks.coffee @@ -2,15 +2,13 @@ exit = (mode, continuation = null) -> mode.exit() continuation?() - false Marks = activateCreateMode: -> mode = new Mode name: "create-mark" indicator: "Create mark?" - keypress: -> false - keyup: -> false + suppressAllKeyboardEvents: true keydown: (event) -> keyChar = KeyboardUtils.getKeyChar(event) if /[A-Z]/.test keyChar @@ -27,17 +25,14 @@ Marks = scrollX: window.scrollX, scrollY: window.scrollY exit mode, -> HUD.showForDuration "Created local mark '#{keyChar}'.", 1000 - else if event.shiftKey - false - else + else if not event.shiftKey exit mode activateGotoMode: -> mode = new Mode name: "goto-mark" indicator: "Go to mark?" - keypress: -> false - keyup: -> false + suppressAllKeyboardEvents: true keydown: (event) -> keyChar = KeyboardUtils.getKeyChar(event) if /[A-Z]/.test keyChar @@ -55,9 +50,7 @@ Marks = HUD.showForDuration "Jumped to local mark '#{keyChar}'", 1000 else HUD.showForDuration "Local mark not set: '#{keyChar}'.", 1000 - else if event.shiftKey - false - else + else if not event.shiftKey exit mode root = exports ? window diff --git a/content_scripts/mode.coffee b/content_scripts/mode.coffee index f631b4cd..b2019ef9 100644 --- a/content_scripts/mode.coffee +++ b/content_scripts/mode.coffee @@ -47,6 +47,15 @@ class Mode @id = "#{@name}-#{@count}" @log "activate:", @id + # If options.suppressAllKeyboardEvents is truthy, then all keyboard events are suppressed. This avoids + # the need for modes which block all keyboard events to 1) provide handlers for all keyboard events, + # and 2) worry about their return value. + if options.suppressAllKeyboardEvents + for type in [ "keydown", "keypress", "keyup" ] + do (type) -> + handler = options[type] + options[type] = (event) -> handler? event; false + @push keydown: @options.keydown || null keypress: @options.keypress || null -- cgit v1.2.3 From e79fe062bc84b1530ec266a12f9323aa53cb89e4 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Wed, 3 Jun 2015 12:02:47 +0100 Subject: Fix tests for #1713... ... which is weird, because the tests are passing here. Let's see what Travis makes of this. --- content_scripts/mode.coffee | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'content_scripts') diff --git a/content_scripts/mode.coffee b/content_scripts/mode.coffee index b2019ef9..cbcc15f7 100644 --- a/content_scripts/mode.coffee +++ b/content_scripts/mode.coffee @@ -50,11 +50,11 @@ class Mode # If options.suppressAllKeyboardEvents is truthy, then all keyboard events are suppressed. This avoids # the need for modes which block all keyboard events to 1) provide handlers for all keyboard events, # and 2) worry about their return value. - if options.suppressAllKeyboardEvents + if @options.suppressAllKeyboardEvents for type in [ "keydown", "keypress", "keyup" ] - do (type) -> - handler = options[type] - options[type] = (event) -> handler? event; false + do (type) => + handler = @options[type] + @options[type] = (event) -> handler? event; false @push keydown: @options.keydown || null -- cgit v1.2.3 From 291e7fd67de9e1c4bd0bc5048ab7344424f19b30 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Wed, 3 Jun 2015 15:27:45 +0100 Subject: Re-implement Marks, incl `` binding. --- content_scripts/marks.coffee | 94 +++++++++++++++++++++------------- content_scripts/mode.coffee | 2 +- content_scripts/vimium_frontend.coffee | 17 ++++-- 3 files changed, 71 insertions(+), 42 deletions(-) (limited to 'content_scripts') diff --git a/content_scripts/marks.coffee b/content_scripts/marks.coffee index 8ba45fd4..fab509a7 100644 --- a/content_scripts/marks.coffee +++ b/content_scripts/marks.coffee @@ -1,57 +1,77 @@ -exit = (mode, continuation = null) -> - mode.exit() - continuation?() - Marks = + mode: null + previousPosition: null + + exit: (continuation = null) -> + @mode?.exit() + @mode = null + continuation?() + + # This returns the key which is used for storing mark locations in localStorage. + getLocationKey: (keyChar) -> + "vimiumMark|#{window.location.href.split('#')[0]}|#{keyChar}" + + showMessage: (message, keyChar) -> + HUD.showForDuration "#{message} \"#{keyChar}\".", 1000 + activateCreateMode: -> - mode = new Mode + @mode = new Mode name: "create-mark" - indicator: "Create mark?" + indicator: "Create mark..." suppressAllKeyboardEvents: true - keydown: (event) -> - keyChar = KeyboardUtils.getKeyChar(event) - if /[A-Z]/.test keyChar - exit mode, -> + keypress: (event) => + keyChar = String.fromCharCode event.charCode + # If is depressed, then it's a global mark, otherwise it's a local mark. This is consistent + # vim's [A-Z] for global marks, [a-z] for local marks. However, it also admits other non-Latin + # characters. + if event.shiftKey + @exit => chrome.runtime.sendMessage handler: 'createMark' markName: keyChar scrollX: window.scrollX scrollY: window.scrollY - , -> HUD.showForDuration "Created global mark '#{keyChar}'.", 1000 - else if /[a-z]/.test keyChar - [baseLocation, sep, hash] = window.location.href.split '#' - localStorage["vimiumMark|#{baseLocation}|#{keyChar}"] = JSON.stringify - scrollX: window.scrollX, - scrollY: window.scrollY - exit mode, -> HUD.showForDuration "Created local mark '#{keyChar}'.", 1000 - else if not event.shiftKey - exit mode - - activateGotoMode: -> - mode = new Mode + , => @showMessage "Created global mark", keyChar + else + @exit => @markPosition keyChar + + markPosition: (keyChar = null) -> + markString = JSON.stringify scrollX: window.scrollX, scrollY: window.scrollY + if keyChar? + localStorage[@getLocationKey keyChar] = markString + @showMessage "Created local mark", keyChar + else + @previousPosition = markString + + activateGotoMode: (registryEntry) -> + # We pick off the last character of the key sequence used to launch this command. Usually this is just "`". + # We then use that character, so together usually the sequence "``", to jump back to the previous + # position. The "previous position" is recorded below, and is registered via @markPosition() elsewhere + # for various other jump-like commands. + previousPositionKey = registryEntry.key[registryEntry.key.length-1..] + @mode = new Mode name: "goto-mark" - indicator: "Go to mark?" + indicator: "Go to mark..." suppressAllKeyboardEvents: true - keydown: (event) -> - keyChar = KeyboardUtils.getKeyChar(event) - if /[A-Z]/.test keyChar - exit mode, -> + keypress: (event) => + keyChar = String.fromCharCode event.charCode + if event.shiftKey + @exit -> chrome.runtime.sendMessage handler: 'gotoMark' markName: keyChar - else if /[a-z]/.test keyChar - [baseLocation, sep, hash] = window.location.href.split '#' - markString = localStorage["vimiumMark|#{baseLocation}|#{keyChar}"] - exit mode, -> + else + markString = + if keyChar == previousPositionKey then @previousPosition else localStorage[@getLocationKey keyChar] + @exit => if markString? - mark = JSON.parse markString - window.scrollTo mark.scrollX, mark.scrollY - HUD.showForDuration "Jumped to local mark '#{keyChar}'", 1000 + @markPosition() + position = JSON.parse markString + window.scrollTo position.scrollX, position.scrollY + @showMessage "Jumped to local mark", keyChar else - HUD.showForDuration "Local mark not set: '#{keyChar}'.", 1000 - else if not event.shiftKey - exit mode + @showMessage "Local mark not set", keyChar root = exports ? window root.Marks = Marks diff --git a/content_scripts/mode.coffee b/content_scripts/mode.coffee index cbcc15f7..22b6120f 100644 --- a/content_scripts/mode.coffee +++ b/content_scripts/mode.coffee @@ -54,7 +54,7 @@ class Mode for type in [ "keydown", "keypress", "keyup" ] do (type) => handler = @options[type] - @options[type] = (event) -> handler? event; false + @options[type] = (event) => handler? event; @stopBubblingAndFalse @push keydown: @options.keydown || null diff --git a/content_scripts/vimium_frontend.coffee b/content_scripts/vimium_frontend.coffee index 7ad75514..9ff9b6db 100644 --- a/content_scripts/vimium_frontend.coffee +++ b/content_scripts/vimium_frontend.coffee @@ -241,9 +241,10 @@ unregisterFrame = -> tab_is_closing: DomUtils.isTopFrame() executePageCommand = (request) -> + commandType = request.command.split(".")[0] # Vomnibar commands are handled in the tab's main/top frame. They are handled even if Vimium is otherwise # disabled in the frame. - if request.command.split(".")[0] == "Vomnibar" + if commandType == "Vomnibar" if DomUtils.isTopFrame() # We pass the frameId from request. That's the frame which originated the request, so that's the frame # which should receive the focus when the vomnibar closes. @@ -254,7 +255,9 @@ executePageCommand = (request) -> # All other commands are handled in their frame (but only if Vimium is enabled). return unless frameId == request.frameId and isEnabledForUrl - if request.registryEntry.passCountToFunction + if commandType == "Marks" + Utils.invokeCommandString request.command, [request.registryEntry] + else if request.registryEntry.passCountToFunction Utils.invokeCommandString(request.command, [request.count]) else Utils.invokeCommandString(request.command) for i in [0...request.count] @@ -299,8 +302,12 @@ window.focusThisFrame = do -> setTimeout (-> highlightedFrameElement.remove()), 200 extend window, - scrollToBottom: -> Scroller.scrollTo "y", "max" - scrollToTop: -> Scroller.scrollTo "y", 0 + scrollToBottom: -> + Marks.markPosition() + Scroller.scrollTo "y", "max" + scrollToTop: -> + Marks.markPosition() + Scroller.scrollTo "y", 0 scrollToLeft: -> Scroller.scrollTo "x", 0 scrollToRight: -> Scroller.scrollTo "x", "max" scrollUp: -> Scroller.scrollBy "y", -1 * Settings.get("scrollStepSize") @@ -861,6 +868,7 @@ window.getFindModeQuery = (backwards) -> findModeQuery.parsedQuery findAndFocus = (backwards) -> + Marks.markPosition() query = getFindModeQuery backwards findModeQueryHasResults = @@ -1007,6 +1015,7 @@ findModeRestoreSelection = (range = findModeInitialRange) -> # Enters find mode. Returns the new find-mode instance. window.enterFindMode = (options = {}) -> + Marks.markPosition() # Save the selection, so performFindInPlace can restore it. findModeSaveSelection() findModeQuery = rawQuery: "" -- cgit v1.2.3 From 8dab334fa2fde9d4815ce0a12c0d2ab9dd44ff05 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Wed, 3 Jun 2015 16:15:31 +0100 Subject: Also set previous position for before global mark movement. --- content_scripts/mode.coffee | 3 +-- content_scripts/vimium_frontend.coffee | 4 +++- 2 files changed, 4 insertions(+), 3 deletions(-) (limited to 'content_scripts') diff --git a/content_scripts/mode.coffee b/content_scripts/mode.coffee index 22b6120f..86d3e011 100644 --- a/content_scripts/mode.coffee +++ b/content_scripts/mode.coffee @@ -52,8 +52,7 @@ class Mode # and 2) worry about their return value. if @options.suppressAllKeyboardEvents for type in [ "keydown", "keypress", "keyup" ] - do (type) => - handler = @options[type] + do (handler = @options[type]) => @options[type] = (event) => handler? event; @stopBubblingAndFalse @push diff --git a/content_scripts/vimium_frontend.coffee b/content_scripts/vimium_frontend.coffee index 9ff9b6db..fec0dae2 100644 --- a/content_scripts/vimium_frontend.coffee +++ b/content_scripts/vimium_frontend.coffee @@ -266,7 +266,9 @@ executePageCommand = (request) -> setScrollPosition = (scrollX, scrollY) -> if (scrollX > 0 || scrollY > 0) - DomUtils.documentReady(-> window.scrollTo(scrollX, scrollY)) + DomUtils.documentReady -> + Marks.markPosition() + window.scrollTo scrollX, scrollY # # Called from the backend in order to change frame focus. -- cgit v1.2.3 From e005d3e5b7a06a949138109b2c57555f9a5db2b9 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Wed, 3 Jun 2015 18:39:03 +0100 Subject: Refactor (to setPreviousPosition) to clarify Marks logic. --- content_scripts/marks.coffee | 25 +++++++++++++------------ content_scripts/vimium_frontend.coffee | 10 +++++----- 2 files changed, 18 insertions(+), 17 deletions(-) (limited to 'content_scripts') diff --git a/content_scripts/marks.coffee b/content_scripts/marks.coffee index fab509a7..9c6b1458 100644 --- a/content_scripts/marks.coffee +++ b/content_scripts/marks.coffee @@ -12,6 +12,12 @@ Marks = getLocationKey: (keyChar) -> "vimiumMark|#{window.location.href.split('#')[0]}|#{keyChar}" + getMarkString: -> + JSON.stringify scrollX: window.scrollX, scrollY: window.scrollY + + setPreviousPosition: -> + @previousPosition = @getMarkString() + showMessage: (message, keyChar) -> HUD.showForDuration "#{message} \"#{keyChar}\".", 1000 @@ -34,21 +40,16 @@ Marks = scrollY: window.scrollY , => @showMessage "Created global mark", keyChar else - @exit => @markPosition keyChar - - markPosition: (keyChar = null) -> - markString = JSON.stringify scrollX: window.scrollX, scrollY: window.scrollY - if keyChar? - localStorage[@getLocationKey keyChar] = markString - @showMessage "Created local mark", keyChar - else - @previousPosition = markString + @exit => + markString = JSON.stringify scrollX: window.scrollX, scrollY: window.scrollY + localStorage[@getLocationKey keyChar] = @getMarkString() + @showMessage "Created local mark", keyChar activateGotoMode: (registryEntry) -> # We pick off the last character of the key sequence used to launch this command. Usually this is just "`". # We then use that character, so together usually the sequence "``", to jump back to the previous - # position. The "previous position" is recorded below, and is registered via @markPosition() elsewhere - # for various other jump-like commands. + # position. The "previous position" is recorded below, and is registered via @setPreviousPosition() + # elsewhere for various other jump-like commands. previousPositionKey = registryEntry.key[registryEntry.key.length-1..] @mode = new Mode name: "goto-mark" @@ -66,7 +67,7 @@ Marks = if keyChar == previousPositionKey then @previousPosition else localStorage[@getLocationKey keyChar] @exit => if markString? - @markPosition() + @setPreviousPosition() position = JSON.parse markString window.scrollTo position.scrollX, position.scrollY @showMessage "Jumped to local mark", keyChar diff --git a/content_scripts/vimium_frontend.coffee b/content_scripts/vimium_frontend.coffee index fec0dae2..f644ea35 100644 --- a/content_scripts/vimium_frontend.coffee +++ b/content_scripts/vimium_frontend.coffee @@ -267,7 +267,7 @@ executePageCommand = (request) -> setScrollPosition = (scrollX, scrollY) -> if (scrollX > 0 || scrollY > 0) DomUtils.documentReady -> - Marks.markPosition() + Marks.setPreviousPosition() window.scrollTo scrollX, scrollY # @@ -305,10 +305,10 @@ window.focusThisFrame = do -> extend window, scrollToBottom: -> - Marks.markPosition() + Marks.setPreviousPosition() Scroller.scrollTo "y", "max" scrollToTop: -> - Marks.markPosition() + Marks.setPreviousPosition() Scroller.scrollTo "y", 0 scrollToLeft: -> Scroller.scrollTo "x", 0 scrollToRight: -> Scroller.scrollTo "x", "max" @@ -870,7 +870,7 @@ window.getFindModeQuery = (backwards) -> findModeQuery.parsedQuery findAndFocus = (backwards) -> - Marks.markPosition() + Marks.setPreviousPosition() query = getFindModeQuery backwards findModeQueryHasResults = @@ -1017,7 +1017,7 @@ findModeRestoreSelection = (range = findModeInitialRange) -> # Enters find mode. Returns the new find-mode instance. window.enterFindMode = (options = {}) -> - Marks.markPosition() + Marks.setPreviousPosition() # Save the selection, so performFindInPlace can restore it. findModeSaveSelection() findModeQuery = rawQuery: "" -- cgit v1.2.3 From 6720f4c5a511e5e7b39e6a59e989353f7fb09792 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Wed, 3 Jun 2015 18:57:39 +0100 Subject: Add mark commands to README, simple refactoring. --- content_scripts/marks.coffee | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) (limited to 'content_scripts') diff --git a/content_scripts/marks.coffee b/content_scripts/marks.coffee index 9c6b1458..73c019da 100644 --- a/content_scripts/marks.coffee +++ b/content_scripts/marks.coffee @@ -31,19 +31,17 @@ Marks = # If is depressed, then it's a global mark, otherwise it's a local mark. This is consistent # vim's [A-Z] for global marks, [a-z] for local marks. However, it also admits other non-Latin # characters. - if event.shiftKey - @exit => - chrome.runtime.sendMessage - handler: 'createMark' - markName: keyChar - scrollX: window.scrollX - scrollY: window.scrollY - , => @showMessage "Created global mark", keyChar - else - @exit => - markString = JSON.stringify scrollX: window.scrollX, scrollY: window.scrollY - localStorage[@getLocationKey keyChar] = @getMarkString() - @showMessage "Created local mark", keyChar + @exit => + if event.shiftKey + chrome.runtime.sendMessage + handler: 'createMark' + markName: keyChar + scrollX: window.scrollX + scrollY: window.scrollY + , => @showMessage "Created global mark", keyChar + else + localStorage[@getLocationKey keyChar] = @getMarkString() + @showMessage "Created local mark", keyChar activateGotoMode: (registryEntry) -> # We pick off the last character of the key sequence used to launch this command. Usually this is just "`". -- cgit v1.2.3 From 457a976421107f9a67ac43090b0370366427bce1 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Wed, 3 Jun 2015 19:13:17 +0100 Subject: Add mark commands to README, more simple refactoring. --- content_scripts/marks.coffee | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) (limited to 'content_scripts') diff --git a/content_scripts/marks.coffee b/content_scripts/marks.coffee index 73c019da..971feadf 100644 --- a/content_scripts/marks.coffee +++ b/content_scripts/marks.coffee @@ -54,16 +54,15 @@ Marks = indicator: "Go to mark..." suppressAllKeyboardEvents: true keypress: (event) => - keyChar = String.fromCharCode event.charCode - if event.shiftKey - @exit -> + @exit => + keyChar = String.fromCharCode event.charCode + if event.shiftKey chrome.runtime.sendMessage handler: 'gotoMark' markName: keyChar - else - markString = - if keyChar == previousPositionKey then @previousPosition else localStorage[@getLocationKey keyChar] - @exit => + else + markString = + if keyChar == previousPositionKey then @previousPosition else localStorage[@getLocationKey keyChar] if markString? @setPreviousPosition() position = JSON.parse markString -- cgit v1.2.3 From 1c4daf625949451178a349e93811b04a00472835 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Wed, 3 Jun 2015 19:15:54 +0100 Subject: Modes, better comments. --- content_scripts/mode.coffee | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'content_scripts') diff --git a/content_scripts/mode.coffee b/content_scripts/mode.coffee index 86d3e011..9105fabb 100644 --- a/content_scripts/mode.coffee +++ b/content_scripts/mode.coffee @@ -48,8 +48,8 @@ class Mode @log "activate:", @id # If options.suppressAllKeyboardEvents is truthy, then all keyboard events are suppressed. This avoids - # the need for modes which block all keyboard events to 1) provide handlers for all keyboard events, - # and 2) worry about their return value. + # the need for modes which block all keyboard events 1) to provide handlers for all keyboard events, + # and 2) to worry about their return values. if @options.suppressAllKeyboardEvents for type in [ "keydown", "keypress", "keyup" ] do (handler = @options[type]) => -- cgit v1.2.3 From b356e50677e1ae7ba68e27b1b70e59744e129627 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Thu, 4 Jun 2015 13:37:40 +0100 Subject: Exit marks modes on escape. --- content_scripts/marks.coffee | 2 ++ 1 file changed, 2 insertions(+) (limited to 'content_scripts') diff --git a/content_scripts/marks.coffee b/content_scripts/marks.coffee index 971feadf..fc30849a 100644 --- a/content_scripts/marks.coffee +++ b/content_scripts/marks.coffee @@ -25,6 +25,7 @@ Marks = @mode = new Mode name: "create-mark" indicator: "Create mark..." + exitOnEscape: true suppressAllKeyboardEvents: true keypress: (event) => keyChar = String.fromCharCode event.charCode @@ -52,6 +53,7 @@ Marks = @mode = new Mode name: "goto-mark" indicator: "Go to mark..." + exitOnEscape: true suppressAllKeyboardEvents: true keypress: (event) => @exit => -- cgit v1.2.3 From bc4f3ecac81fd8f174b8c3ad92a0998ada8f7992 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Thu, 4 Jun 2015 15:14:10 +0100 Subject: Global marks; focus and only scroll in the main frame. --- content_scripts/vimium_frontend.coffee | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) (limited to 'content_scripts') diff --git a/content_scripts/vimium_frontend.coffee b/content_scripts/vimium_frontend.coffee index f644ea35..dc083e6c 100644 --- a/content_scripts/vimium_frontend.coffee +++ b/content_scripts/vimium_frontend.coffee @@ -147,7 +147,7 @@ initializePreDomReady = -> focusFrame: (request) -> if (frameId == request.frameId) then focusThisFrame request refreshCompletionKeys: refreshCompletionKeys getScrollPosition: -> scrollX: window.scrollX, scrollY: window.scrollY - setScrollPosition: (request) -> setScrollPosition request.scrollX, request.scrollY + setScrollPosition: setScrollPosition executePageCommand: executePageCommand currentKeyQueue: (request) -> keyQueue = request.keyQueue @@ -264,11 +264,15 @@ executePageCommand = (request) -> refreshCompletionKeys(request) -setScrollPosition = (scrollX, scrollY) -> - if (scrollX > 0 || scrollY > 0) - DomUtils.documentReady -> - Marks.setPreviousPosition() - window.scrollTo scrollX, scrollY +# Set the scroll position (but only in the main frame). Some pages (like Facebook) get confused if you set +# the scroll position in all frames. +setScrollPosition = ({ scrollX, scrollY }) -> + if DomUtils.isTopFrame() + if scrollX > 0 or scrollY > 0 + DomUtils.documentReady -> + window.focus() + Marks.setPreviousPosition() + window.scrollTo scrollX, scrollY # # Called from the backend in order to change frame focus. -- cgit v1.2.3 From 9600d258cbadd80568e37637974172e7755ca28c Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Thu, 4 Jun 2015 16:07:57 +0100 Subject: Global marks; only handle messages in main frame. On sites with several frames (e.g. Facebook), if we allow all of the frames to handle the setScrollPosition and showHudForDuration messages, then the focus ends up in an arbitrary frame. And, on Facebook, say, Vimium ends up broken until the focus is returned to the main frame. So, we only handle these messages in the main frame. --- content_scripts/vimium_frontend.coffee | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) (limited to 'content_scripts') diff --git a/content_scripts/vimium_frontend.coffee b/content_scripts/vimium_frontend.coffee index dc083e6c..033bb2b3 100644 --- a/content_scripts/vimium_frontend.coffee +++ b/content_scripts/vimium_frontend.coffee @@ -142,7 +142,7 @@ initializePreDomReady = -> window.removeEventListener "focus", onFocus requestHandlers = - showHUDforDuration: (request) -> HUD.showForDuration request.text, request.duration + showHUDforDuration: handleShowHUDforDuration toggleHelpDialog: (request) -> toggleHelpDialog(request.dialogHtml, request.frameId) focusFrame: (request) -> if (frameId == request.frameId) then focusThisFrame request refreshCompletionKeys: refreshCompletionKeys @@ -264,13 +264,16 @@ executePageCommand = (request) -> refreshCompletionKeys(request) -# Set the scroll position (but only in the main frame). Some pages (like Facebook) get confused if you set -# the scroll position in all frames. +handleShowHUDforDuration = ({ text, duration }) -> + if DomUtils.isTopFrame() + DomUtils.documentReady -> HUD.showForDuration text, duration + setScrollPosition = ({ scrollX, scrollY }) -> if DomUtils.isTopFrame() - if scrollX > 0 or scrollY > 0 - DomUtils.documentReady -> - window.focus() + DomUtils.documentReady -> + window.focus() + document.body.focus() + if 0 < scrollX or 0 < scrollY Marks.setPreviousPosition() window.scrollTo scrollX, scrollY -- cgit v1.2.3 From 2473475367411fdeb051a6752d0059e337ad09e3 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Thu, 4 Jun 2015 16:23:54 +0100 Subject: Use only ' and ` for jumping to previous position. As suggested by @mrmr1993 in #1716. --- content_scripts/marks.coffee | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) (limited to 'content_scripts') diff --git a/content_scripts/marks.coffee b/content_scripts/marks.coffee index fc30849a..5bb35f1f 100644 --- a/content_scripts/marks.coffee +++ b/content_scripts/marks.coffee @@ -1,7 +1,8 @@ Marks = + previousPositionRegisters: [ "`", "'" ] + localRegisters: {} mode: null - previousPosition: null exit: (continuation = null) -> @mode?.exit() @@ -16,7 +17,8 @@ Marks = JSON.stringify scrollX: window.scrollX, scrollY: window.scrollY setPreviousPosition: -> - @previousPosition = @getMarkString() + markString = @getMarkString() + @localRegisters[reg] = markString for reg in @previousPositionRegisters showMessage: (message, keyChar) -> HUD.showForDuration "#{message} \"#{keyChar}\".", 1000 @@ -45,11 +47,6 @@ Marks = @showMessage "Created local mark", keyChar activateGotoMode: (registryEntry) -> - # We pick off the last character of the key sequence used to launch this command. Usually this is just "`". - # We then use that character, so together usually the sequence "``", to jump back to the previous - # position. The "previous position" is recorded below, and is registered via @setPreviousPosition() - # elsewhere for various other jump-like commands. - previousPositionKey = registryEntry.key[registryEntry.key.length-1..] @mode = new Mode name: "goto-mark" indicator: "Go to mark..." @@ -63,8 +60,7 @@ Marks = handler: 'gotoMark' markName: keyChar else - markString = - if keyChar == previousPositionKey then @previousPosition else localStorage[@getLocationKey keyChar] + markString = @localRegisters[keyChar] ? localStorage[@getLocationKey keyChar] if markString? @setPreviousPosition() position = JSON.parse markString -- cgit v1.2.3 From 89c3ab075994de9b11952784eb5752da445df576 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Thu, 4 Jun 2015 16:36:00 +0100 Subject: Unwind passing key and registry entry for marks. As suggested by @mrmr1993 in #1716. --- content_scripts/vimium_frontend.coffee | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) (limited to 'content_scripts') diff --git a/content_scripts/vimium_frontend.coffee b/content_scripts/vimium_frontend.coffee index 033bb2b3..3055ecea 100644 --- a/content_scripts/vimium_frontend.coffee +++ b/content_scripts/vimium_frontend.coffee @@ -255,9 +255,7 @@ executePageCommand = (request) -> # All other commands are handled in their frame (but only if Vimium is enabled). return unless frameId == request.frameId and isEnabledForUrl - if commandType == "Marks" - Utils.invokeCommandString request.command, [request.registryEntry] - else if request.registryEntry.passCountToFunction + if request.registryEntry.passCountToFunction Utils.invokeCommandString(request.command, [request.count]) else Utils.invokeCommandString(request.command) for i in [0...request.count] -- cgit v1.2.3 From 50980718d848ca324b6fca0568e2fa7203d51d52 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Fri, 5 Jun 2015 05:04:56 +0100 Subject: Fix event suppression for Marks keyboard events. --- content_scripts/mode.coffee | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) (limited to 'content_scripts') diff --git a/content_scripts/mode.coffee b/content_scripts/mode.coffee index 9105fabb..ffabc111 100644 --- a/content_scripts/mode.coffee +++ b/content_scripts/mode.coffee @@ -48,12 +48,11 @@ class Mode @log "activate:", @id # If options.suppressAllKeyboardEvents is truthy, then all keyboard events are suppressed. This avoids - # the need for modes which block all keyboard events 1) to provide handlers for all keyboard events, - # and 2) to worry about their return values. + # the need for modes which suppress all keyboard events 1) to provide handlers for all of those events, + # or 2) to worry about event suppression and event-handler return values. if @options.suppressAllKeyboardEvents for type in [ "keydown", "keypress", "keyup" ] - do (handler = @options[type]) => - @options[type] = (event) => handler? event; @stopBubblingAndFalse + @options[type] = @alwaysSuppressEvent @options[type] @push keydown: @options.keydown || null @@ -179,6 +178,13 @@ class Mode # case), because they do not need to be concerned with the value they yield. alwaysContinueBubbling: handlerStack.alwaysContinueBubbling + # Shorthand for an event handler which always suppresses event propagation. + alwaysSuppressEvent: (handler = null) -> + (event) => + handler? event + DomUtils.suppressPropagation event + @stopBubblingAndFalse + # Activate a new instance of this mode, together with all of its original options (except its main # keybaord-event handlers; these will be recreated). cloneMode: -> -- cgit v1.2.3 From a143d2c81a9faaf383a05b0dae2f232db85959a2 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Fri, 5 Jun 2015 06:35:26 +0100 Subject: Global marks; fix indentation. --- content_scripts/marks.coffee | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) (limited to 'content_scripts') diff --git a/content_scripts/marks.coffee b/content_scripts/marks.coffee index 5bb35f1f..3b5814da 100644 --- a/content_scripts/marks.coffee +++ b/content_scripts/marks.coffee @@ -36,15 +36,15 @@ Marks = # characters. @exit => if event.shiftKey - chrome.runtime.sendMessage - handler: 'createMark' - markName: keyChar - scrollX: window.scrollX - scrollY: window.scrollY - , => @showMessage "Created global mark", keyChar + chrome.runtime.sendMessage + handler: 'createMark' + markName: keyChar + scrollX: window.scrollX + scrollY: window.scrollY + , => @showMessage "Created global mark", keyChar else - localStorage[@getLocationKey keyChar] = @getMarkString() - @showMessage "Created local mark", keyChar + localStorage[@getLocationKey keyChar] = @getMarkString() + @showMessage "Created local mark", keyChar activateGotoMode: (registryEntry) -> @mode = new Mode -- cgit v1.2.3 From 8eda31fb236b9506d577a25908d4e5334d3289d5 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Fri, 5 Jun 2015 07:07:02 +0100 Subject: Global marks; global marks are always recorded for the top frame only. With global marks, we may later create a new tab when the mark is used. When doing so, we should always be using the URL and scroll position of the top frame within the tab. For example, if a global mark is created from within the Hangouts frame of Google's Inbox and the mark is later used, then we should create a new tab for Inbox's URL and scroll position, not for the Hangout's URL and scroll position. --- content_scripts/marks.coffee | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) (limited to 'content_scripts') diff --git a/content_scripts/marks.coffee b/content_scripts/marks.coffee index 3b5814da..f569e1ea 100644 --- a/content_scripts/marks.coffee +++ b/content_scripts/marks.coffee @@ -36,11 +36,14 @@ Marks = # characters. @exit => if event.shiftKey + # We record the current scroll position, but only if this is the top frame within the tab. + # Otherwise, we'll fetch the scroll position of the top frame from the background page later. + [ scrollX, scrollY ] = [ window.scrollX, window.scrollY ] if DomUtils.isTopFrame() chrome.runtime.sendMessage handler: 'createMark' markName: keyChar - scrollX: window.scrollX - scrollY: window.scrollY + scrollX: scrollX + scrollY: scrollY , => @showMessage "Created global mark", keyChar else localStorage[@getLocationKey keyChar] = @getMarkString() -- cgit v1.2.3 From 83fc0e58f6139ff5a1ae37fc300ea647da079171 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Fri, 5 Jun 2015 07:14:50 +0100 Subject: Global marks; always treat ` and ' as local marks. On some keyboards (who knows?) "`" or "'" could be shift keys. In this case, with the previous implementation, these would be treated as global marks and `` would be unusable. This commit fixes that problem. --- content_scripts/marks.coffee | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) (limited to 'content_scripts') diff --git a/content_scripts/marks.coffee b/content_scripts/marks.coffee index f569e1ea..067d05a8 100644 --- a/content_scripts/marks.coffee +++ b/content_scripts/marks.coffee @@ -23,6 +23,12 @@ Marks = showMessage: (message, keyChar) -> HUD.showForDuration "#{message} \"#{keyChar}\".", 1000 + # If is depressed, then it's a global mark, otherwise it's a local mark. This is consistent + # vim's [A-Z] for global marks and [a-z] for local marks. However, it also admits other non-Latin + # characters. The exceptions are "`" and "'", which are always considered local marks. + isGlobalMark: (event, keyChar) -> + event.shiftKey and keyChar not in @previousPositionRegisters + activateCreateMode: -> @mode = new Mode name: "create-mark" @@ -31,11 +37,8 @@ Marks = suppressAllKeyboardEvents: true keypress: (event) => keyChar = String.fromCharCode event.charCode - # If is depressed, then it's a global mark, otherwise it's a local mark. This is consistent - # vim's [A-Z] for global marks, [a-z] for local marks. However, it also admits other non-Latin - # characters. @exit => - if event.shiftKey + if @isGlobalMark event, keyChar # We record the current scroll position, but only if this is the top frame within the tab. # Otherwise, we'll fetch the scroll position of the top frame from the background page later. [ scrollX, scrollY ] = [ window.scrollX, window.scrollY ] if DomUtils.isTopFrame() @@ -58,7 +61,7 @@ Marks = keypress: (event) => @exit => keyChar = String.fromCharCode event.charCode - if event.shiftKey + if @isGlobalMark event, keyChar chrome.runtime.sendMessage handler: 'gotoMark' markName: keyChar -- cgit v1.2.3