Merge pull request #1716 from smblott-github/global-marks

Improved global marks

3 files changed, 117 insertions, 53 deletions

diff --git a/content_scripts/marks.coffee b/content_scripts/marks.coffee
index 316ab951..067d05a8 100644
--- a/content_scripts/marks.coffee
+++ b/content_scripts/marks.coffee
@@ -1,45 +1,79 @@
-root = window.Marks = {}
-
-root.activateCreateMode = ->
-  handlerStack.push keydown: (e) ->
-    keyChar = KeyboardUtils.getKeyChar(event)
-    return unless keyChar isnt ""
-
-    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
-
-    @remove()
-
-    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
+
+Marks =
+  previousPositionRegisters: [ "`", "'" ]
+  localRegisters: {}
+  mode: 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}"
+
+  getMarkString: ->
+    JSON.stringify scrollX: window.scrollX, scrollY: window.scrollY
+
+  setPreviousPosition: ->
+    markString = @getMarkString()
+    @localRegisters[reg] = markString for reg in @previousPositionRegisters
+
+  showMessage: (message, keyChar) ->
+    HUD.showForDuration "#{message} \"#{keyChar}\".", 1000
+
+  # If <Shift> 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"
+      indicator: "Create mark..."
+      exitOnEscape: true
+      suppressAllKeyboardEvents: true
+      keypress: (event) =>
+        keyChar = String.fromCharCode event.charCode
+        @exit =>
+          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()
+            chrome.runtime.sendMessage
+              handler: 'createMark'
+              markName: keyChar
+              scrollX: scrollX
+              scrollY: scrollY
+            , => @showMessage "Created global mark", keyChar
+          else
+            localStorage[@getLocationKey keyChar] = @getMarkString()
+            @showMessage "Created local mark", keyChar
+
+  activateGotoMode: (registryEntry) ->
+    @mode = new Mode
+      name: "goto-mark"
+      indicator: "Go to mark..."
+      exitOnEscape: true
+      suppressAllKeyboardEvents: true
+      keypress: (event) =>
+        @exit =>
+          keyChar = String.fromCharCode event.charCode
+          if @isGlobalMark event, keyChar
+            chrome.runtime.sendMessage
+              handler: 'gotoMark'
+              markName: keyChar
+          else
+            markString = @localRegisters[keyChar] ? localStorage[@getLocationKey keyChar]
+            if markString?
+              @setPreviousPosition()
+              position = JSON.parse markString
+              window.scrollTo position.scrollX, position.scrollY
+              @showMessage "Jumped to local mark", keyChar
+            else
+              @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 f631b4cd..ffabc111 100644
--- a/content_scripts/mode.coffee
+++ b/content_scripts/mode.coffee
@@ -47,6 +47,13 @@ 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 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" ]
+        @options[type] = @alwaysSuppressEvent @options[type]
+
     @push
       keydown: @options.keydown || null
       keypress: @options.keypress || null
@@ -171,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: ->
diff --git a/content_scripts/vimium_frontend.coffee b/content_scripts/vimium_frontend.coffee
index 7ad75514..3055ecea 100644
--- a/content_scripts/vimium_frontend.coffee
+++ b/content_scripts/vimium_frontend.coffee
@@ -142,12 +142,12 @@ 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
     getScrollPosition: -> scrollX: window.scrollX, scrollY: window.scrollY
-    setScrollPosition: (request) -> setScrollPosition request.scrollX, request.scrollY
+    setScrollPosition: setScrollPosition
     executePageCommand: executePageCommand
     currentKeyQueue: (request) ->
       keyQueue = request.keyQueue
@@ -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.
@@ -261,9 +262,18 @@ executePageCommand = (request) ->
 
   refreshCompletionKeys(request)
 
-setScrollPosition = (scrollX, scrollY) ->
-  if (scrollX > 0 || scrollY > 0)
-    DomUtils.documentReady(-> window.scrollTo(scrollX, scrollY))
+handleShowHUDforDuration = ({ text, duration }) ->
+  if DomUtils.isTopFrame()
+    DomUtils.documentReady -> HUD.showForDuration text, duration
+
+setScrollPosition = ({ scrollX, scrollY }) ->
+  if DomUtils.isTopFrame()
+    DomUtils.documentReady ->
+      window.focus()
+      document.body.focus()
+      if 0 < scrollX or 0 < scrollY
+        Marks.setPreviousPosition()
+        window.scrollTo scrollX, scrollY
 
 #
 # Called from the backend in order to change frame focus.
@@ -299,8 +309,12 @@ window.focusThisFrame = do ->
       setTimeout (-> highlightedFrameElement.remove()), 200
 
 extend window,
-  scrollToBottom: -> Scroller.scrollTo "y", "max"
-  scrollToTop: -> Scroller.scrollTo "y", 0
+  scrollToBottom: ->
+    Marks.setPreviousPosition()
+    Scroller.scrollTo "y", "max"
+  scrollToTop: ->
+    Marks.setPreviousPosition()
+    Scroller.scrollTo "y", 0
   scrollToLeft: -> Scroller.scrollTo "x", 0
   scrollToRight: -> Scroller.scrollTo "x", "max"
   scrollUp: -> Scroller.scrollBy "y", -1 * Settings.get("scrollStepSize")
@@ -861,6 +875,7 @@ window.getFindModeQuery = (backwards) ->
     findModeQuery.parsedQuery
 
 findAndFocus = (backwards) ->
+  Marks.setPreviousPosition()
   query = getFindModeQuery backwards
 
   findModeQueryHasResults =
@@ -1007,6 +1022,7 @@ findModeRestoreSelection = (range = findModeInitialRange) ->
 
 # Enters find mode.  Returns the new find-mode instance.
 window.enterFindMode = (options = {}) ->
+  Marks.setPreviousPosition()
   # Save the selection, so performFindInPlace can restore it.
   findModeSaveSelection()
   findModeQuery = rawQuery: ""