Improve Find mode search start position

By default, Firefox starts searching after the end of the first selection range,
or from the top of the page if there are no selection ranges. This means that if
you search for "vim", a match for "vim" further up or down the page might be
selected, even though the word "vim" might already be onscreen, because of the
current selection (or lack thereof).

Vim starts searching in a similar way: After the cursor. The difference is that
in Vim, the cursor is _always_ onscreen. When you scroll, the cursor is "pushed
along" by the edges of the screen. This way, it is always very easy to tell
where you are going to start your search from. In Firefox, though, using the `/`
command can be quite disorienting.

Another Vim example: Let's say I searched for "vim" again (pressing `/vim` in
Vim). I then go through a few matches using `n`. Then I come to a block of text
that mentions "vim" _lots_ of times, and I realize that I'm not interested in
that block at all. Then it is easier to scroll (or "jump") past that block
rather than spamming `n` over and over (or trying to guess an appropriate
count). When doing the same in Firefox, though, after having scrolled past the
mentioned block of text, the next `n` keypress will scroll up right back to the
last match and continue the search there. This is pretty annoying to me.

While there's the Caret mode (which few people use) and the possibility to
select text in Firefox, there's usually no concept of a caret, and the caret
does not follow the scroll; it stays where it is, possibly offscreen. So where
should we start searching from, really?

This commit implements the following logic:

If there's a visible selection on screen, start searching after that point. If
not, start searching from the top of the current viewport. This works much more
intuitively. Note: Elements with `position: fixed;` are excluded when
determining "the top of the viewport".

Inspired by vimium/issues#1471.

documentation/options.md

@@ -215,6 +215,31 @@ Controls whether [counts] are enabled or not.
 
 [counts]: commands.md#counts
 
+### `find_from_top_of_viewport`
+
+Toggles whether the various find commands are Vim-style or Firefox
+default-style.
+
+Disable this pref if you want `/` to work more like `<c-f>` and `n`/`N` to work
+more like `<f3>`/`<s-f3>`.
+
+If there is selected text on the page, Firefox starts searching after that.
+VimFx does so too, but only if the selection is currently _visible_ (inside the
+current viewport).
+
+If there _isn’t_ selected text on the page, Firefox starts searching from the
+top of the page. VimFx instead starts searching from the top of the current
+viewport.
+
+The VimFx behavior is designed to be less disorienting. It is also similar to
+how searching in Vim works. Again, you can return to the Firefox default
+behavior (if you prefer that) by disabling this pref.
+
+One of the main benefits of the VimFx behavior is that you can scroll past a
+block of the text with lots of search matches and then continue going through
+matches with `n` after that block, without having to spam `n` lots and lots of
+times.
+
 ### `ignore_ctrl_alt`
 
 This option is enabled by default on Windows, and disabled otherwise.

extension/lib/commands-frame.coffee

@@ -29,6 +29,7 @@ translate = require('./l10n')
 utils = require('./utils')
 viewportUtils = require('./viewport')
 
+{FORWARD, BACKWARD} = SelectionManager
 {isProperLink, isTextInputElement, isTypingElement, isContentEditable} = utils
 
 XULDocument = Ci.nsIDOMXULDocument
@@ -479,6 +480,51 @@ commands.move_focus = ({vim, direction}) ->
     utils.focusElement(nextInput, {select: true})
     return true
 
+# This is an attempt to enhance Firefox’s native “Find in page” functionality.
+# Firefox starts searching after the end of the first selection range, or from
+# the top of the page if there are no selection ranges. If there are frames, the
+# top-most document in DOM order with selections seems to be used.
+#
+# Replace the current selection with one single range. (Searching clears the
+# previous selection anyway.) That single range is either the first visible
+# range, or a newly created (and collapsed) one at the top of the viewport. This
+# way we can control where Firefox searches from.
+commands.find_from_top_of_viewport = ({vim, direction}) ->
+  viewport = viewportUtils.getWindowViewport(vim.content)
+
+  range = viewportUtils.getFirstVisibleRange(vim.content, viewport)
+  if range
+    window = range.startContainer.ownerGlobal
+    selection = window.getSelection()
+    utils.clearSelectionDeep(vim.content)
+    window.focus()
+    # When the next match is in another frame than the current selection (A),
+    # Firefox won’t clear that selection before making a match selection (B) in
+    # the other frame. When searching again, selection B is cleared because
+    # selection A appears further up the viewport. This causes us to search
+    # _again_ from selection A, rather than selection B. In effect, we get stuck
+    # re-selecting selection B over and over. Therefore, collapse the range
+    # first, in case Firefox doesn’t.
+    range.collapse()
+    selection.addRange(range)
+    # Collapsing the range causes backwards search to keep re-selecting the same
+    # match. Therefore, move it one character back.
+    selection.modify('move', 'backward', 'character') if direction == BACKWARD
+    return
+
+  result = viewportUtils.getFirstVisibleText(vim.content, viewport)
+  return unless result
+  [textNode, offset] = result
+
+  utils.clearSelectionDeep(vim.content)
+  window = textNode.ownerGlobal
+  window.focus()
+  range = window.document.createRange()
+  range.setStart(textNode, offset)
+  range.setEnd(textNode, offset)
+  selection = window.getSelection()
+  selection.addRange(range)
+
 commands.esc = (args) ->
   {vim} = args
   commands.blur_active_element(args)

extension/lib/commands.coffee

@@ -32,9 +32,11 @@ hints = require('./hints')
 prefs = require('./prefs')
 translate = require('./l10n')
 utils = require('./utils')
+SelectionManager = require('./selection')
 viewportUtils = require('./viewport')
 
 {ContentClick} = Cu.import('resource:///modules/ContentClick.jsm', {})
+{FORWARD, BACKWARD} = SelectionManager
 
 commands = {}
 
@@ -632,23 +634,31 @@ commands.element_text_copy = ({vim}) ->
 
 findStorage = {lastSearchString: ''}
 
+helper_find_from_top_of_viewport = (vim, direction, callback) ->
+  if vim.options.find_from_top_of_viewport
+    vim._run('find_from_top_of_viewport', {direction}, callback)
+  else
+    callback()
+
 helper_find = ({highlight, linksOnly = false}, {vim}) ->
   helpSearchInput = help.getSearchInput(vim.window)
   if helpSearchInput
     helpSearchInput.select()
     return
 
   helper_mark_last_scroll_position(vim)
-  findBar = vim.window.gBrowser.getFindBar()
+  helper_find_from_top_of_viewport(vim, FORWARD, ->
+    findBar = vim.window.gBrowser.getFindBar()
 
-  mode = if linksOnly then findBar.FIND_LINKS else findBar.FIND_NORMAL
-  findBar.startFind(mode)
-  utils.focusElement(findBar._findField, {select: true})
+    mode = if linksOnly then findBar.FIND_LINKS else findBar.FIND_NORMAL
+    findBar.startFind(mode)
+    utils.focusElement(findBar._findField, {select: true})
 
-  return if linksOnly
-  return unless highlightButton = findBar.getElement('highlight')
-  if highlightButton.checked != highlight
-    highlightButton.click()
+    return if linksOnly
+    return unless highlightButton = findBar.getElement('highlight')
+    if highlightButton.checked != highlight
+      highlightButton.click()
+  )
 
 commands.find = helper_find.bind(null, {highlight: false})
 
@@ -661,15 +671,18 @@ helper_find_again = (direction, {vim}) ->
   if findStorage.lastSearchString.length == 0
     vim.notify(translate('notification.find_again.none'))
     return
+
   helper_mark_last_scroll_position(vim)
-  findBar._findField.value = findStorage.lastSearchString
-  findBar.onFindAgainCommand(direction)
-  message = findBar._findStatusDesc.textContent
-  vim.notify(message) if message
+  helper_find_from_top_of_viewport(vim, direction, ->
+    findBar._findField.value = findStorage.lastSearchString
+    findBar.onFindAgainCommand(not direction)
+    message = findBar._findStatusDesc.textContent
+    vim.notify(message) if message
+  )
 
-commands.find_next     = helper_find_again.bind(null, false)
+commands.find_next     = helper_find_again.bind(null, FORWARD)
 
-commands.find_previous = helper_find_again.bind(null, true)
+commands.find_previous = helper_find_again.bind(null, BACKWARD)
 
 
 

extension/lib/defaults.coffee

@@ -161,6 +161,7 @@ advanced_options =
   'notify_entered_keys':                true
   'prevent_target_blank':               true
   'counts_enabled':                     true
+  'find_from_top_of_viewport':          true
   'ignore_ctrl_alt':                    (Services.appinfo.OS == 'WINNT')
   'prevent_autofocus_modes':            'normal'
   'config_file_directory':              ''

extension/lib/utils.coffee

@@ -316,6 +316,13 @@ simulateMouseEvents = (element, sequence) ->
 area = (element) ->
   return element.clientWidth * element.clientHeight
 
+checkElementOrAncestor = (element, fn) ->
+  window = element.ownerGlobal
+  while element.parentElement
+    return true if fn(element)
+    element = element.parentElement
+  return false
+
 clearSelectionDeep = (window) ->
   # The selection might be `null` in hidden frames.
   selection = window.getSelection()
@@ -369,6 +376,9 @@ insertText = (input, value) ->
 isDetached = (element) ->
   return not element.ownerDocument?.documentElement?.contains?(element)
 
+isNonEmptyTextNode = (node) ->
+  return node.nodeType == 3 and node.data.trim() != ''
+
 isPositionFixed = (element) ->
   computedStyle = element.ownerGlobal.getComputedStyle(element)
   return computedStyle?.getPropertyValue('position') == 'fixed'
@@ -566,13 +576,15 @@ module.exports = {
   simulateMouseEvents
 
   area
+  checkElementOrAncestor
   clearSelectionDeep
   containsDeep
   createBox
   getRootElement
   injectTemporaryPopup
   insertText
   isDetached
+  isNonEmptyTextNode
   isPositionFixed
   querySelectorAllDeep
   setAttributes

extension/lib/viewport.coffee

@@ -46,6 +46,53 @@ adjustRectToViewport = (rect, viewport) ->
     height, width, area
   }
 
+getAllRangesInsideViewport = (window, viewport, offset = {left: 0, top: 0}) ->
+  selection = window.getSelection()
+  {rangeCount} = selection
+  ranges = []
+
+  if rangeCount > 0
+    {header, headerBottom, footer, footerTop} =
+      getFixedHeaderAndFooter(window, viewport)
+
+    # Many sites use `text-indent: -9999px;` or similar to hide text intended
+    # for screen readers only. That text can still be selected and found by
+    # searching, though. Therefore, we have to allow selection ranges that are
+    # within the viewport vertically but not horizontally, even though they
+    # actually are outside the viewport. Otherwise you won’t be able to press
+    # `n` to get past those elements (instead, `n` would start over from the top
+    # of the viewport).
+    for index in [0...rangeCount]
+      range = selection.getRangeAt(index)
+      continue if range.collapsed
+      rect = range.getBoundingClientRect()
+      if (rect.top >= headerBottom - MINIMUM_EDGE_DISTANCE and
+          rect.bottom <= footerTop + MINIMUM_EDGE_DISTANCE) or
+         header.contains(range.commonAncestorContainer) or
+         footer.contains(range.commonAncestorContainer)
+        adjustedRect = {
+          left: offset.left + rect.left
+          top: offset.top + rect.top
+          right: offset.left + rect.right
+          bottom: offset.top + rect.bottom
+          width: rect.width
+          height: rect.height
+        }
+        ranges.push({range, rect: adjustedRect})
+
+  for frame in window.frames
+    {viewport: frameViewport, offset: frameOffset} =
+      getFrameViewport(frame.frameElement, viewport) ? {}
+    continue unless frameViewport
+    newOffset = {
+      left: offset.left + frameOffset.left
+      top: offset.top + frameOffset.top
+    }
+    frameRanges = getAllRangesInsideViewport(frame, frameViewport, newOffset)
+    ranges.push(frameRanges...)
+
+  return ranges
+
 getFirstNonWhitespace = (element) ->
   window = element.ownerGlobal
   viewport = getWindowViewport(window)
@@ -63,9 +110,7 @@ getFirstNonWhitespace = (element) ->
 getFirstVisibleOffset = (textNode, viewport) ->
   {length} = textNode.data
   return null if length == 0
-  [header] = getFixedHeaderAndFooter(textNode.ownerGlobal, viewport)
-  headerBottom =
-    if header then header.getBoundingClientRect().bottom else viewport.top
+  {headerBottom} = getFixedHeaderAndFooter(textNode.ownerGlobal, viewport)
   [nonMatch, match] = utils.bisect(0, length - 1, (offset) ->
     range = textNode.ownerDocument.createRange()
     # Using a zero-width range sometimes gives a bad rect, so make it span one
@@ -80,6 +125,36 @@ getFirstVisibleOffset = (textNode, viewport) ->
   )
   return match
 
+getFirstVisibleRange = (window, viewport) ->
+  ranges = getAllRangesInsideViewport(window, viewport)
+  first = null
+  for item in ranges
+    if not first or item.rect.top < first.rect.top
+      first = item
+  return if first then first.range else null
+
+getFirstVisibleText = (window, viewport) ->
+  for element in window.document.getElementsByTagName('*')
+    rect = element.getBoundingClientRect()
+    continue unless isInsideViewport(rect, viewport)
+
+    if element.contentWindow and
+       not utils.checkElementOrAncestor(element, utils.isPositionFixed)
+      {viewport: frameViewport} = getFrameViewport(element, viewport) ? {}
+      continue unless frameViewport
+      result = getFirstVisibleText(element.contentWindow, frameViewport)
+      return result if result
+      continue
+
+    continue unless Array.some(element.childNodes, utils.isNonEmptyTextNode)
+
+    continue if utils.checkElementOrAncestor(element, utils.isPositionFixed)
+
+    result = getFirstNonWhitespace(element)
+    return result if result
+
+  return null
+
 # Adapted from Firefox’s source code for `<space>` scrolling (which is where the
 # arbitrary constants below come from).
 #
@@ -116,7 +191,7 @@ getFixedHeaderAndFooter = (window) ->
         footer = candidate
         footerTop = rect.top
 
-  return [header, footer]
+  return {header, headerBottom, footer, footerTop}
 
 getFrameViewport = (frame, parentViewport) ->
   rect = frame.getBoundingClientRect()
@@ -161,11 +236,7 @@ getFrameViewport = (frame, parentViewport) ->
 getViewportCappedClientHeight = (element) ->
   window = element.ownerGlobal
   viewport = getWindowViewport(window)
-  [header, footer] = getFixedHeaderAndFooter(window)
-  headerBottom =
-    if header then header.getBoundingClientRect().bottom else viewport.top
-  footerTop =
-    if footer then footer.getBoundingClientRect().bottom else viewport.bottom
+  {headerBottom, footerTop} = getFixedHeaderAndFooter(window)
   return Math.min(element.clientHeight, footerTop - headerBottom)
 
 getWindowViewport = (window) ->
@@ -217,8 +288,11 @@ scroll = (element, args) ->
 module.exports = {
   MINIMUM_EDGE_DISTANCE
   adjustRectToViewport
+  getAllRangesInsideViewport
   getFirstNonWhitespace
   getFirstVisibleOffset
+  getFirstVisibleRange
+  getFirstVisibleText
   getFixedHeaderAndFooter
   getFrameViewport
   getViewportCappedClientHeight