From 3671382a6a3c14d1f834b6ebcf10605921201c53 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Tue, 1 Mar 2016 07:02:27 +0000 Subject: Refactor hints; fix exit sequence. Previously, the exit sequence when a link was "clicked" was spread over several functions with several callbacks. This made it difficult to verify that the correct actions were happening in the correct order. Indeed, they weren't in at least one case (we were still showing hints while "waiting for enter"). This fixes that by putting all of the various deactivation orders into one place, `@activateLink()`, and simplifies `@deactivateMode()` accordingle. --- content_scripts/link_hints.coffee | 82 ++++++++++++++++++++------------------- 1 file changed, 42 insertions(+), 40 deletions(-) (limited to 'content_scripts') diff --git a/content_scripts/link_hints.coffee b/content_scripts/link_hints.coffee index c8f9b6f9..b72dd6d3 100644 --- a/content_scripts/link_hints.coffee +++ b/content_scripts/link_hints.coffee @@ -369,7 +369,9 @@ class LinkHintsMode if linksMatched.length == 0 @deactivateMode() else if linksMatched.length == 1 - @activateLink linksMatched[0], keyResult.delay ? 0, keyResult.waitForEnter and Settings.get "waitForEnterForFilteredHints" + keyResult.delay ?= 0 + keyResult.waitForEnter ?= false + @activateLink linksMatched[0], keyResult else @hideMarker marker for marker in hintMarkers @showMarker matched, @markerMatcher.hintKeystrokeQueue.length for matched in linksMatched @@ -377,26 +379,30 @@ class LinkHintsMode # # When only one link hint remains, this function activates it in the appropriate way. # - activateLink: (@matchedLink, delay = 0, waitForEnter = false) -> + activateLink: (@matchedLink, {delay, waitForEnter} = {}) -> + @removeHintMarkers() clickEl = @matchedLink.clickableItem - if (DomUtils.isSelectable(clickEl)) - DomUtils.simulateSelect(clickEl) - @deactivateMode delay - else - # TODO figure out which other input elements should not receive focus - if (clickEl.nodeName.toLowerCase() == "input" and clickEl.type not in ["button", "submit"]) - clickEl.focus() - linkActivator = => - @linkActivator(clickEl) + linkActivator = => + @deactivateMode() + if DomUtils.isSelectable clickEl + DomUtils.simulateSelect clickEl + else + # TODO: Are there any other input elements which should not receive focus? + if clickEl.nodeName.toLowerCase() == "input" and clickEl.type not in ["button", "submit"] + clickEl.focus() + @linkActivator clickEl LinkHints.activateModeWithQueue() if @mode is OPEN_WITH_QUEUE - if waitForEnter - new WaitForEnter @matchedLink.rect, => @deactivateMode 0, linkActivator - else - @deactivateMode delay, => - DomUtils.flashRect @matchedLink.rect - linkActivator() + if waitForEnter? and waitForEnter + new WaitForEnter @matchedLink.rect, linkActivator + else if delay? and 0 < delay + # Install a mode to block keyboard events if the user is still typing. The intention is to prevent the + # user from inadvertently launching Vimium commands when typing the link text. + new TypingProtector delay, @matchedLink?.rect, linkActivator + else + DomUtils.flashRect @matchedLink.rect + linkActivator() # # Shows the marker, highlighting matchingCharCount characters. @@ -411,27 +417,19 @@ class LinkHintsMode hideMarker: (linkMarker) -> linkMarker.style.display = "none" - deactivateMode: (delay = 0, callback = null) -> - deactivate = => - DomUtils.removeElement @hintMarkerContainingDiv if @hintMarkerContainingDiv - @hintMarkerContainingDiv = null - @markerMatcher = null - @isActive = false - @hintMode?.exit() - @hintMode = null - @onExit?() - @onExit = null - @tabCount = 0 - callback?() - - if delay - # Install a mode to block keyboard events if the user is still typing. The intention is to prevent the - # user from inadvertently launching Vimium commands when typing the link text. - new TypingProtector delay, @matchedLink?.rect, deactivate - else - # We invoke deactivate() directly (instead of setting a timeout of 0) so that deactivateMode() can be - # tested synchronously. - deactivate() + deactivateMode: -> + @removeHintMarkers() + @markerMatcher = null + @isActive = false + @hintMode?.exit() + @hintMode = null + @onExit?() + @onExit = null + @tabCount = 0 + + removeHintMarkers: -> + DomUtils.removeElement @hintMarkerContainingDiv if @hintMarkerContainingDiv + @hintMarkerContainingDiv = null # Use characters for hints, and do not filter links by their text. class AlphabetHints @@ -576,7 +574,11 @@ class FilterHints @activeHintMarker = linksMatched[tabCount] @activeHintMarker?.classList.add "vimiumActiveHintMarker" - { linksMatched: linksMatched, delay: delay, waitForEnter: 0 < delay } + { + linksMatched: linksMatched + delay: delay + waitForEnter: 0 < delay and Settings.get "waitForEnterForFilteredHints" + } pushKeyChar: (keyChar, keydownKeyChar) -> # For filtered hints, we *always* use the keyChar value from keypress, because there is no obvious and @@ -686,7 +688,7 @@ class WaitForEnter extends Mode callback() DomUtils.suppressEvent event else - true + true flashEl = DomUtils.addFlashRect rect @onExit -> DomUtils.removeElement flashEl -- cgit v1.2.3 From 0761521e45af560453abb10b35dfae410ae8bce9 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Tue, 1 Mar 2016 07:06:47 +0000 Subject: Refactor hints; remove legacy code. Previously (quite some time ago) we reused the LinkHints object. But for some time it's been a class, and we never reuse instances. Therefore, we can remove the code related to resetting the object's state. --- content_scripts/link_hints.coffee | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) (limited to 'content_scripts') diff --git a/content_scripts/link_hints.coffee b/content_scripts/link_hints.coffee index b72dd6d3..d82b3b70 100644 --- a/content_scripts/link_hints.coffee +++ b/content_scripts/link_hints.coffee @@ -43,8 +43,6 @@ class LinkHintsMode mode: undefined # Function that does the appropriate action on the selected link. linkActivator: undefined - # Lock to ensure only one instance runs at a time. - isActive: false # The link-hints "mode" (in the key-handler, indicator sense). hintMode: null # Call this function on exit (if defined). @@ -55,7 +53,6 @@ class LinkHintsMode constructor: (mode = OPEN_IN_CURRENT_TAB, onExit = (->)) -> # we need documentElement to be ready in order to append links return unless document.documentElement - @isActive = true elements = @getVisibleClickableElements() # For these modes, we filter out those elements which don't have an HREF (since there's nothing we can do @@ -88,7 +85,7 @@ class LinkHintsMode keypress: @onKeyPressInMode.bind this, hintMarkers @hintMode.onExit => - @deactivateMode() if @isActive + @deactivateMode() @hintMode.onExit onExit @setOpenLinkMode mode @@ -322,7 +319,7 @@ class LinkHintsMode keyup: (event) => if event.keyCode == keyCode handlerStack.remove() - @setOpenLinkMode previousMode if @isActive + @setOpenLinkMode previousMode true # Continue bubbling the event. # For some (unknown) reason, we don't always receive the keyup event needed to remove this handler. @@ -419,13 +416,8 @@ class LinkHintsMode deactivateMode: -> @removeHintMarkers() - @markerMatcher = null - @isActive = false @hintMode?.exit() - @hintMode = null @onExit?() - @onExit = null - @tabCount = 0 removeHintMarkers: -> DomUtils.removeElement @hintMarkerContainingDiv if @hintMarkerContainingDiv -- cgit v1.2.3 From 9c37e4430b1f4fec946cc0080fb1d944a8ab7d87 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Tue, 1 Mar 2016 07:23:59 +0000 Subject: Refactor hints; remove trailing whitespace. Somehow, --- content_scripts/link_hints.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'content_scripts') diff --git a/content_scripts/link_hints.coffee b/content_scripts/link_hints.coffee index d82b3b70..81d305a7 100644 --- a/content_scripts/link_hints.coffee +++ b/content_scripts/link_hints.coffee @@ -680,7 +680,7 @@ class WaitForEnter extends Mode callback() DomUtils.suppressEvent event else - true + true flashEl = DomUtils.addFlashRect rect @onExit -> DomUtils.removeElement flashEl -- cgit v1.2.3 From 66e1f46b8865ff3ae98037590fcb149d2e98e873 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Tue, 1 Mar 2016 07:27:51 +0000 Subject: Refactor hints; consistent variable naming. While we're changing this code, we can renamed the parameter here to be consistent with its naming elsewhere. --- content_scripts/link_hints.coffee | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'content_scripts') diff --git a/content_scripts/link_hints.coffee b/content_scripts/link_hints.coffee index 81d305a7..54d2980e 100644 --- a/content_scripts/link_hints.coffee +++ b/content_scripts/link_hints.coffee @@ -376,9 +376,9 @@ class LinkHintsMode # # When only one link hint remains, this function activates it in the appropriate way. # - activateLink: (@matchedLink, {delay, waitForEnter} = {}) -> + activateLink: (linkMatched, {delay, waitForEnter} = {}) -> @removeHintMarkers() - clickEl = @matchedLink.clickableItem + clickEl = linkMatched.clickableItem linkActivator = => @deactivateMode() @@ -392,13 +392,13 @@ class LinkHintsMode LinkHints.activateModeWithQueue() if @mode is OPEN_WITH_QUEUE if waitForEnter? and waitForEnter - new WaitForEnter @matchedLink.rect, linkActivator + new WaitForEnter linkMatched.rect, linkActivator else if delay? and 0 < delay # Install a mode to block keyboard events if the user is still typing. The intention is to prevent the # user from inadvertently launching Vimium commands when typing the link text. - new TypingProtector delay, @matchedLink?.rect, linkActivator + new TypingProtector delay, linkMatched?.rect, linkActivator else - DomUtils.flashRect @matchedLink.rect + DomUtils.flashRect linkMatched.rect linkActivator() # -- cgit v1.2.3 From 7dfcd123191ff9b7deb9059e1d454d425254125c Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Tue, 1 Mar 2016 09:58:40 +0000 Subject: Refactor hints; add `userMightOverType`. Previously, we set a variable `delay` and then did some logical gymnastics to get the correct effect. However, in fact, all we care about is whether the user might over-type the links text. So changing to using that as a Boolean flag greatly simplifies the logic. And we lose about 10 LoC. --- content_scripts/link_hints.coffee | 33 ++++++++++----------------------- 1 file changed, 10 insertions(+), 23 deletions(-) (limited to 'content_scripts') diff --git a/content_scripts/link_hints.coffee b/content_scripts/link_hints.coffee index 54d2980e..cfacb862 100644 --- a/content_scripts/link_hints.coffee +++ b/content_scripts/link_hints.coffee @@ -361,14 +361,11 @@ class LinkHintsMode DomUtils.suppressEvent event updateVisibleMarkers: (hintMarkers, tabCount = 0) -> - keyResult = @markerMatcher.getMatchingHints hintMarkers, tabCount - linksMatched = keyResult.linksMatched + {linksMatched, userMightOverType} = @markerMatcher.getMatchingHints hintMarkers, tabCount if linksMatched.length == 0 @deactivateMode() else if linksMatched.length == 1 - keyResult.delay ?= 0 - keyResult.waitForEnter ?= false - @activateLink linksMatched[0], keyResult + @activateLink linksMatched[0], userMightOverType ? false else @hideMarker marker for marker in hintMarkers @showMarker matched, @markerMatcher.hintKeystrokeQueue.length for matched in linksMatched @@ -376,7 +373,7 @@ class LinkHintsMode # # When only one link hint remains, this function activates it in the appropriate way. # - activateLink: (linkMatched, {delay, waitForEnter} = {}) -> + activateLink: (linkMatched, userMightOverType=false) -> @removeHintMarkers() clickEl = linkMatched.clickableItem @@ -391,12 +388,12 @@ class LinkHintsMode @linkActivator clickEl LinkHints.activateModeWithQueue() if @mode is OPEN_WITH_QUEUE - if waitForEnter? and waitForEnter + if userMightOverType and Settings.get "waitForEnterForFilteredHints" new WaitForEnter linkMatched.rect, linkActivator - else if delay? and 0 < delay - # Install a mode to block keyboard events if the user is still typing. The intention is to prevent the - # user from inadvertently launching Vimium commands when typing the link text. - new TypingProtector delay, linkMatched?.rect, linkActivator + else if userMightOverType + # Block keyboard events while the user is still typing. The intention is to prevent the user from + # inadvertently launching Vimium commands when (over-)typing the link text. + new TypingProtector 200, linkMatched?.rect, linkActivator else DomUtils.flashRect linkMatched.rect linkActivator() @@ -546,19 +543,12 @@ class FilterHints @filterLinkHints hintMarkers getMatchingHints: (hintMarkers, tabCount = 0) -> - delay = 0 - # At this point, linkTextKeystrokeQueue and hintKeystrokeQueue have been updated to reflect the latest # input. use them to filter the link hints accordingly. matchString = @hintKeystrokeQueue.join "" linksMatched = @filterLinkHints hintMarkers linksMatched = linksMatched.filter (linkMarker) -> linkMarker.hintString.startsWith matchString - if linksMatched.length == 1 && @hintKeystrokeQueue.length == 0 and 0 < @linkTextKeystrokeQueue.length - # In filter mode, people tend to type out words past the point needed for a unique match. Hence we - # should avoid passing control back to command mode immediately after a match is found. - delay = 200 - # Visually highlight of the active hint (that is, the one that will be activated if the user # types ). tabCount = ((linksMatched.length * Math.abs tabCount) + tabCount) % linksMatched.length @@ -566,11 +556,8 @@ class FilterHints @activeHintMarker = linksMatched[tabCount] @activeHintMarker?.classList.add "vimiumActiveHintMarker" - { - linksMatched: linksMatched - delay: delay - waitForEnter: 0 < delay and Settings.get "waitForEnterForFilteredHints" - } + linksMatched: linksMatched + userMightOverType: @hintKeystrokeQueue.length == 0 and 0 < @linkTextKeystrokeQueue.length pushKeyChar: (keyChar, keydownKeyChar) -> # For filtered hints, we *always* use the keyChar value from keypress, because there is no obvious and -- cgit v1.2.3 From de6c623b65f072650e70c8c5acf8c9d809bc64f5 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Fri, 4 Mar 2016 06:44:36 +0000 Subject: Refactor hints; no need to guard against no rect. - The check for whether a rect is defined is only used in one of the three cases. So we don't need it. - Also, better veriable name. --- content_scripts/link_hints.coffee | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) (limited to 'content_scripts') diff --git a/content_scripts/link_hints.coffee b/content_scripts/link_hints.coffee index cfacb862..57fec611 100644 --- a/content_scripts/link_hints.coffee +++ b/content_scripts/link_hints.coffee @@ -393,7 +393,7 @@ class LinkHintsMode else if userMightOverType # Block keyboard events while the user is still typing. The intention is to prevent the user from # inadvertently launching Vimium commands when (over-)typing the link text. - new TypingProtector 200, linkMatched?.rect, linkActivator + new TypingProtector 200, linkMatched.rect, linkActivator else DomUtils.flashRect linkMatched.rect linkActivator() @@ -633,24 +633,22 @@ class TypingProtector extends Mode constructor: (delay, rect, callback) -> @timer = Utils.setTimeout delay, => @exit() - handler = (event) => + resetExitTimer = (event) => clearTimeout @timer @timer = Utils.setTimeout delay, => @exit() super name: "hint/typing-protector" suppressAllKeyboardEvents: true - keydown: handler - keypress: handler - - if rect - # We keep a "flash" overlay active while the user is typing; this provides visual feeback that something - # has been selected. - flashEl = DomUtils.addFlashRect rect - @onExit -> DomUtils.removeElement flashEl + keydown: resetExitTimer + keypress: resetExitTimer @onExit callback + # We keep a "flash" overlay active while the user is typing; this provides visual feeback that something + # has been selected. + flashEl = DomUtils.addFlashRect rect + @onExit -> DomUtils.removeElement flashEl class WaitForEnter extends Mode constructor: (rect, callback) -> -- cgit v1.2.3