From ef5e236676be4190bf368bf4abe35fa60d59774d Mon Sep 17 00:00:00 2001 From: Simon Lydell Date: Sat, 14 Nov 2015 21:28:34 +0100 Subject: [PATCH] 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. See #605. --- extension/lib/commands-frame.coffee | 8 +-- extension/lib/events-frame.coffee | 53 ++++++----------- extension/lib/scrollable-elements.coffee | 73 ++++++++++++++++++++++++ extension/lib/utils.coffee | 21 +++++++ extension/lib/vim-frame.coffee | 14 ++--- 5 files changed, 121 insertions(+), 48 deletions(-) create mode 100644 extension/lib/scrollable-elements.coffee diff --git a/extension/lib/commands-frame.coffee b/extension/lib/commands-frame.coffee index a8f13a8..f10b6f7 100644 --- a/extension/lib/commands-frame.coffee +++ b/extension/lib/commands-frame.coffee @@ -49,16 +49,16 @@ commands.scroll = (args) -> activeElement # If the currently focused element isnât scrollable, scroll the largest # scrollable element instead, which usually means ``. - when vim.state.largestScrollableElement - { largestScrollableElement } = vim.state + when vim.state.scrollableElements.largest + { scrollableElements } = vim.state # In quirks mode (when the page lacks a doctype) `` is considered # the root element rather than ``. The 'overflow' event is triggered # for `` though (_not_ ``!). - if largestScrollableElement == document.documentElement and + if scrollableElements.largest == document.documentElement and document.compatMode == 'BackCompat' and document.body? document.body else - largestScrollableElement + scrollableElements.largest else # This point should never be reached, but itâs better to be safe than # sorry. Not being able to scroll is very annoying. Use the best bet. diff --git a/extension/lib/events-frame.coffee b/extension/lib/events-frame.coffee index 3c720a8..0fc73cd 100644 --- a/extension/lib/events-frame.coffee +++ b/extension/lib/events-frame.coffee @@ -40,12 +40,22 @@ class FrameEventManager ) @listen('readystatechange', (event) => - window = @vim.content - # Only handle 'readystatechange' for the top-most window, not for frames, - # and only before any state about the page has been collected. - if window.document.readyState == 'interactive' + target = event.originalTarget + + # When the page starts loading `.readyState` changes to 'interactive'. + return unless target.readyState == 'interactive' + + # If the topmost document starts loading, it means that we have navigated + # to a new page or refreshed the page. + if target == @vim.content.document @vim.resetState() - messageManager.send('locationChange', window.location.href) + messageManager.send('locationChange', @vim.content.location.href) + else + # If the target isnât the topmost document, it means that a frame has + # started loading. Some sites change the `src` attribute of `