Simon Lydell [Thu, 26 Nov 2015 09:51:41 +0000 (10:51 +0100)]
Rework "move tab to window" detection logic
The old detection was too unreliable and didn't catch all cases.
Also fixes a bug where the blacklist wasn't applied when going back or forward
in history to a blacklisted URL.
The long story:
If you move a tab into another window in multi-process, the frame script for the
web page of the tab is kept. That is good, since all state (such as scrollable
elements) is preserved.
In non-multi-process, that is not the case. A new frame script is loaded, so all
state is lost. That is unfortunate, but there's nothing we can do about it. In
non-multi-process, moving a tab to another window then works a bit like closing
the tab and opening a new one in the target winow with the same URL: All VimFx
state is lost, including the current mode. The blacklist is applied. However,
the web page content is the same, with DOM modifications and scroll position
preserved. This is not ideal for VimFx, but OK. Non-multi-process will be
dropped from Firefox not too far into the future anyway.
Let's focus on multi-process instead.
That no new frame script is loaded for the "new" (dragged) tab in the target
window has some interesting consequences. For example, `vim` objects for tabs
are created based on a frame script being loaded. Does this mean that no `vim`
object is created for this tab? It depends.
When a new tab is opened, Firefox might load a frame script for `about:blank`.
When the real web page starts loading, a new frame script is loaded.
The above can depend on whether you're moving (dragging) a tab to another
existing window, or moving a tab to an entirely new window (by right-clicking a
tab and choosing “Move to New Window” or dragging a tab and dropping it outside
a tab bar).
If a frame script for `about:blank` _is_ loaded, a `vim` object is created.
However, Firefox then moves over the web page from the old tab, and replaces the
`.messageManager` of the new tab’s `<browser>`! This means that the `vim` object
cannot receive messages from the just-moved-over web page (coming from the frame
script that was kept from the origin window, as mentioned earlier), because it
has attached its listeners to a message manager object that no longer exists.
In both cases it means that VimFx won't work correctly in those tabs. Therefore,
we need to detect tabs moved to other windows.
There does not seem to be any straight-forward way of detecting that, though.
However, we can abuse the fact the `vim` objects for those tabs will either be
missing or have the wrong `._messageManager` as a way of detecting it anyway.
(Since that isn't the case in non-multi-process, that's why we can't detect it
there.)
After having moved a tab to another window, the web page of the tab loads from
cache. This triggers a 'pageshow' event with `event.persisted === true`, just
like when navigating forward and backward in history. When this happens, the
frame scripts send a message to the UI process, which is caught at window level.
If there's no `vim` object for the `<browser>` that the message came from, or if
`vim._messageManager` is out of date, we know that the tab for the `<browser>`
element has just been moved to another window!
In that case, we want to find the `vim` object for the closed tab in the origin
window (the one that we started dragging, for example) and re-use it for the
"new" tab, in order to keep the current mode etc.
That is done be saving the `vim` object of the last closed tab, by listening for
the 'TabClose' event. It fires before the new tab in the target window is
created.
The whole process is very ugly and a bit complicated, but there's seems to be no
other way, and it does the job: Things work acceptably in non-multi-process and
(seemingly) perfectly in multi-process.
Simon Lydell [Tue, 24 Nov 2015 17:37:40 +0000 (18:37 +0100)]
Implement simple marks
Inspired by Vim and Vimium, but simpler. All they do is to save the current
toplevel scroll position for the current page, letting you return to that point
later. Global marks are intentionally left out: See #626.
Simon Lydell [Sun, 22 Nov 2015 16:28:51 +0000 (17:28 +0100)]
Fix broken updating of the button
When moving from `@vimfx.emit.bind(@vimfx, 'TabSelect', event)` some time, that
code was mistakenly changed to `@vimfx.emit(@vimfx, 'TabSelect', event)` instead
of `@vimfx.emit('TabSelect', event)`
Simon Lydell [Sun, 22 Nov 2015 11:26:14 +0000 (12:26 +0100)]
Fix hints mode not exiting when focusing text input
`marker.type` (which is `undefined`) was used instead of `marker.wrapper.type`
when checking if it is `'text'`. This caused for example `af` not to exit hints
mode when typing the hint for a text input.
Simon Lydell [Sun, 22 Nov 2015 10:03:59 +0000 (11:03 +0100)]
Improve largest scrollable element detection
In commit f405054c a special case was added for the root element:
> Consider the entire page as largest if scrollable
>
> Its area may technically be less than other elements, but if it is scrollable
> that's what you expect to scroll.
However, if you embed such a page in a frame, the page won’t be scrollable
(because it is not the uppermost root element).
This made me realize that it makes the most sense to consider the _uppermost
scrollable element_ the largest. In other words, if a scrollable element
contains another scrollable element (or a frame containing one), the parent
should be considered largest even if the child has greater area.
Simon Lydell [Sun, 22 Nov 2015 08:57:31 +0000 (09:57 +0100)]
Fix being unable to scroll in quirks mode
Alternatively:
Handle quirks mode in a better and more abstracted way.
In quirks mode, `<html>` might receive an 'overflow' event, but it is `<body>`
that is scrollable (and it does _not_ receive an 'overflow' event!) The
`ScrollableElements` class now automatically takes care of this case, storing
`<body>` instead if you try to store `<html>` in quirks mode. This way, only
actually scrollable elements are stored, which makes comparisons such as
`isScrollable` work without having to think about quirks mode. Previously, the
`isScrollable` check in the 'overflow' event handler failed on `<html>` in
quirks mode, making it impossible to scroll on quirky sites, such as Hackernews.
Simon Lydell [Sat, 21 Nov 2015 12:57:04 +0000 (13:57 +0100)]
Fix text input focus problems since commit a197a16
Since that commit, and only in non-multi-process, when focusing a text input for
a second time it got styled as focused, but no blinking text caret appeared. You
could still type into the text input, though.
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.