Simon Lydell [Sat, 21 Nov 2015 10:22:26 +0000 (11:22 +0100)]
Improve scrollable element detection when zoomed
Elements may overflow when zooming in or out. However, the `.scrollHeight` of
the element is not correctly updated when the 'overflow' event occurs, making it
possible for unscrollable elements to slip in. An example is Google Groups.
There, the entire page gets an 'overflow' event when zooming, and by then it
actually looks to be scrollable. When inspecting the element afterwards, though,
it becomes clear that it is not. Well, actually, then it looks like the element
is scrollable by 1px. So this commit also sets a minimum scroll to 5px.
The solution to the zoom problem is to check if the largest scrollable element
really is scrollable before trying to scroll it. If not, update it.
Simon Lydell [Wed, 18 Nov 2015 18:11:02 +0000 (19:11 +0100)]
Simplify scrollable element rejection
One 'pagehide' event is fired for every removed frame, starting with the most
deeply nested one, so there's no need to do a deep containment check for
scrollable elements.
Simon Lydell [Wed, 18 Nov 2015 13:54:20 +0000 (14:54 +0100)]
Enhance `overflow` check for scrollable elements
Checking the computed style for `overflow` proved to be a bit reliable.
Explicitly checking both `overflow-x` and `overflow-y` works better. If both are
set to `hidden` the element is not scrollable.
Simon Lydell [Tue, 17 Nov 2015 08:36:53 +0000 (09:36 +0100)]
Fix state being lost on some pages
... especially the largest scrollable element, usually `<html>`.
Since commit 87ece2ac we use the 'readystatechange' to detect location change in
a tab. While that seems to work fine, resetting state at that point does not.
Apparently, at least on some sites, a 'readystatechange' with `readyState ==
'interative'` is fired _after_ `<html>` has been detected as a scrollable
element, causing that scrollable element to be lost (because of a state reset).
If another scrollable element is detected after that, scrolling the entire page
becomes impossible.
Luckily, the 'pagehide' event seems to be exactly what we're looking for, when
it comes to resetting state. It also has the bonus of firing in frames, removing
the need for the `MutationObserver`s for frames added in commit ef5e2366.
Simon Lydell [Mon, 16 Nov 2015 20:54:21 +0000 (21:54 +0100)]
Fix broken notifications
I got it wrong in commit 340584c8 and believed that only one `<statuspanel>` was
needed per window, but one is actually needed for every _tab._ Luckily, the code
became a lot simpler.
Simon Lydell [Wed, 11 Nov 2015 17:03:40 +0000 (18:03 +0100)]
Rework how notifications are shown
Users found `new Notification(message)` too intrusive. This commit shows
notifications in a `<statuspanel>` instead, like the "URL popup" (shown when
hovering or focusing links), but on the opposite side.
When pressing `n` or `N` and the search matches a link, the URL popup is shown.
Putting the VimFx notification on the opposite side allows both to be visible at
the same time.
Notifications are shown until you click them, press a key or switch tab.
Simon Lydell [Thu, 12 Nov 2015 18:55:08 +0000 (19:55 +0100)]
Properly exit "`gi` mode"
Before, if you pressed `gi`, blurred the text input and then focused a text
input (without using Hints mode, such as using `<tab>` or clicking), you would
still be in "`gi` mode", making `<tab>` only cycle between text inputs.
Simon Lydell [Thu, 12 Nov 2015 08:31:08 +0000 (09:31 +0100)]
Rework `gi` handling of `<tab>`
The "Focus previous/next element" commands turned out to be too intrusive. This
commit removes them, and instead handles `<tab>` and `<s-tab>` especially. Those
keys are handled:
- _only_ in web page content.
- _only_ if in "`gi` mode".
- always "late", taking `event.defaultPrevented` into account.
- not if `<tab>` or `<s-tab>` was part of a command shortcut.
This commit removes some (theoretical) flexibility in order to stay true to our
goal not to interfere with the browser.
Simon Lydell [Sun, 15 Nov 2015 01:19:38 +0000 (02:19 +0100)]
Fix missing hints for links with floated children
Previously, we used `nsIDOMWindowUtils#nodesFromRect()` to get markable element
candidates, because of its speed. However, it turned out that that method does
not return elements with zero area, such as links with floated children. That
caused those links to not get a hint.
This commit switches back to `document.getElementsByTagName('*')`. Due to some
reordering of element filtering there seems to be no difference in performance!
Simon Lydell [Sun, 15 Nov 2015 00:04:10 +0000 (01:04 +0100)]
Fix un-blurrable comment fields on Facebook
We used to only blur focusable elements (`.tabIndex > -1`) to "interfere with
the browser as little as possible." However, this made it impossible to blur
Facebook's comment fields. _Not_ being able to blur things is what interferes.
Simon Lydell [Sat, 14 Nov 2015 23:18:10 +0000 (00:18 +0100)]
Improve count and wrap semantics for tab select/move
When using one of `J`, `K`, `gJ` and `gK` with a count, you usually don't want
to wrap around the tab bar, in case you counted too large a number. Then it's
less confusing what happened if it stops at the edge of the tab bar. This is how
it already works.
However, if you're at one of the ends of the tab bar and cannot move in the
chosen direction, there is no reason to forbid a count. Before this commit we
did, making the command a no-op. Now it is allowed. As an example, you can
select the second to last tab by pressing `2J` if you're currently on the first
tab.
The commands only wrap _once._ In other words, if you're on the first of five
tabs, pressing `6J` takes you back to the first tab (five moves), discarding the
last move instead of wrapping again to the last tab. This is useful if you count
too large a number.
Simon Lydell [Sat, 14 Nov 2015 22:42:22 +0000 (23:42 +0100)]
Handle {,non-}pinned tabs separately in `gJ` and `gK`
Previously, it was possible to move a non-pinned tab into the pinned tabs,
making it pinned, and to move a pinnned tab into the non-pinned tabs, making it
non-pinned.
Now, that's not the case anymore. The first non-pinned tab wraps to the last
tab, and the last tab wraps to the first non-pinned tab, and vice versa for
pinned tabs.
In other words, you can no longer use `gJ` and `gK` to change the pinned state
of a tab. Only `gp` can.
Simon Lydell [Sat, 14 Nov 2015 20:28:34 +0000 (21:28 +0100)]
Fix issues with scrolling and frames
Since commit 4958beb3b we use `MutationObserver`s to keep
`vim.state.scrollableElements` up-to-date when scrollable elements are removed.
However, there are two more ways elements can be removed if the are located
inside a frame:
- If the whole frame is removed.
- If the `src` attribute of the frame is changed.
This caused "TypeError: can't access dead object" errors on gmail.com.
This commit takes care of those two frame cases as well.
Simon Lydell [Sat, 14 Nov 2015 16:04:02 +0000 (17:04 +0100)]
Remove `utils.timeIt(fn)`
It required `fn` to be synchronous. Since the multi-process support, very little
is anymore, making it easier to add `console.time('foo')` and
`console.timeEnd('foo')` where needed.
Simon Lydell [Sat, 14 Nov 2015 15:05:57 +0000 (16:05 +0100)]
Fix quirks after dragging tab to another window
The quirks only happened in multi-process, where a new `vim` instance is not
instantiated for the dragged tab. We already update `.browser` and `.window`.
However, all frame script listeners were still using the message manager of the
old `<browser>`, making the `vim` instance miss out on its frame script
messages. For example, this caused the blacklist not to apply when reloading the
page in a tab dragged to another window. This commit adds the listeners for the
new `<browser>` as well.
Simon Lydell [Sat, 14 Nov 2015 14:20:40 +0000 (15:20 +0100)]
Don't lose state when loading tabs in the background
'locationChange' used to be detected in a rather complicated fashion. While this
worked mostly it had problems with tabs loading in the background.
'locationChange' wouldn't fire until the a background tab was _selected,_
causing any state, such as scrollable elements, collected up to that point to be
lost. It also meant that the blacklist patterns were applied to late, which
could cause unwanted autofocus prevention, for example.
Using the 'readystatechange' inside frame scripts seems to do exactly what we
want. This commit switches to that technique, which also simplifies the code a
lot, removing many slightly hacky parts.
Simon Lydell [Sat, 14 Nov 2015 11:52:50 +0000 (12:52 +0100)]
Keep the largest scrollable element up-to-date
We used to not try to find a new largest scrollable element if the currently
largest scrollable element underflowed (stopped being scrollable), because it
was considered "unlikely". That might be true for "classic static" web pages,
but it is not for JavaScript-heavy sites, such as groups.google.com, which may
replace most of the DOM at any time. Therefore, we now _do_ try to find a new
largest scrollable element when the old stops being scrollable.
The above required changing the storage for the scrollable elements from a
`WeakSet` to a `Set`, in order to be able to enumerate the set. The reason
`WeakSet` was used before was to avoid memory leaks. However, that should not be
a problem, because we now remove elements removed from the DOM from the set,
because of the next paragraph. Also, all state is wiped when the page is left.
Unfortunately, the 'underflow' event is not fired for scrollable elements
removed from the DOM. Therefore that is tracked separately using
`MutationObserver`s.
Simon Lydell [Sat, 14 Nov 2015 10:40:38 +0000 (11:40 +0100)]
Choose which element to scroll more wisely
'overflow' events _are_ triggered for `<html>`, if it overflows. This means that
there is no need to try to detect if `<html>` is scrollable in `commands.scroll`
using `.scrollHeight` and `.clientHeight`; we can simply use
`vim.state.largestScrollableElement`.
The above is not only a code cleanup; it also fixes a bug, mentioned in #605:
On some sites (such as groups.google.com), `<html>` might overflow, but not be
scrollable because of `overflow: hidden;`. The old `.scrollHeight` vs.
`.clientHeight` technique did not catch this, and wrongly tried to scroll
`<html>` when there was a more appropriate scrollable element on the page.
`overflow: hidden;` _is_ taken care of for `vim.state.scrollableElements` and
`vim.state.largestScrollableElement`, though, fixing the problem.
> Revert "Remove unnecessary code in commands.follow{,_focus}"
>
> This reverts commit 1a77a5d62036ee0d6029b36ee3c2ef33355b0d85.
>
> Apparently, I must be wrong. I get a hint marker for the entire page (if it is
> scrollable) after this commit. So confused now.
As noted above, `<html>` (`document.documentElement`) _does_ get 'overflow'
events, and thus ends up in `vim.state.scrollableElements`, so we _should_
exclude `document.documentElement` from getting a marker. There is no need to
deal with quirks mode here, though. In quirks mode, `<body>` is considered the
root element and must be scrolled instead of `<html>`, but 'overflow' events are
still triggered for `<html>`, and _not_ for `<body>`.
Simon Lydell [Wed, 11 Nov 2015 06:51:49 +0000 (07:51 +0100)]
Allow to use numbers as shortcuts, overriding counts
... properly handling the special-case for `0`:
- `0J` is _not_ a count, because a count of zero makes no sense. It first
invokes the `0` command, and then the `J` command.
- `20J` _is_ a count, repeating twenty times. In other words, pressing `2` and
then `0` does _not_ run the `0` command (scroll to the far left) with a count
of two. Instead, it sets the waiting count to twenty.
Simon Lydell [Tue, 10 Nov 2015 18:27:20 +0000 (19:27 +0100)]
Add migration to add `<force>` to `mode.normal.esc`
We already migrate old shortcut customizations to the new 0.6.0 format. However,
I forgot that if the user had customized the shortcut for the esc command, it
most likely needs `<force>` at the beginning as well.
This commit adds `<force>` to every shortcut for that command (if needed).
Ideally this should have been done already in the 0.6.0 release. Now we run the
tiny risk of adding `<force>` to a shortcut that the user actually intended not
to have `<force>` in, but it's worth it: See #577, #571, #582 and #583.
Simon Lydell [Mon, 9 Nov 2015 18:01:39 +0000 (19:01 +0100)]
Silence AMO `Function` warnings
The AMO validator thinks that if `Function` appears anywhere, it is a type of
`eval`. However, we use it to access `Function.prototype.call`. This commit
replaces the `Function` occurances with other methods in order to silence those
warnings.
Commit c1bc315c tried to simplify the compatible Firefox version management by
using `*` as the maximum version. However, that is not allowed when uploading to
AMO.
Simon Lydell [Wed, 4 Nov 2015 16:27:51 +0000 (17:27 +0100)]
Make it possible to blur scrollable elements
Scrollable elements _are_ focusable (both by using `<tab>` and by using VimFx's
`f` commands), but they aren't marked as such by the browser. In order to
interfere as little as possible, VimFx only blurs elements marked as focusable.
This made it impossible to blur scrollable elements, preventing you from
scrolling the entire page again. This commits now also allows scrollable
elements to blurred as a special-case.
Simon Lydell [Tue, 3 Nov 2015 18:46:49 +0000 (19:46 +0100)]
Fix global variable leaks in frame scripts
Apparently, frame scripts for the same `<browser>` but from different add-ons
share the same scope. Previously, bootstrap.coffee used to set up a few global
variables. However, those were then leaked to other add-ons in frame scripts.
Now they are explicitly set in the `scope` of each `require`d module instead.