From 8a33140f484cfe41f6259f7b0387a71d8e3b1f47 Mon Sep 17 00:00:00 2001 From: girst Date: Tue, 5 May 2020 14:18:53 +0200 Subject: [PATCH] Don't crash on pages with out-of-process iframes when fission is enabled This should be considered at most a workaround; oop iframes are completely opaque to VimFx. This not only means that hints are not generated for elements inside them, but also that VimFx can't detect when an input element is focused. Due to the latter, we switch to insert mode whenever we detect that focus is inside an OOP iframe, so text inputs can still be used. During implementation, a bug (https://bugzilla.mozilla.org/1635530) was found (and filed) where activeElement was wrong. This is fixed as of Firefox 78. Given that fission is still disabled by default, earlier verisons aren't affected. Note: lib/utils.coffee::getTopOffset() also accesses frameElement, but that's safe, since this function is only called by simulateMouseEvents(), which only works on elements that were previously reached via hints mode (where we already check for oop iframes). lib/utils.coffee::containsDeep() is also safe, albeit less so. It is called through lib/scrollable-elements.coffee::addChecked() by lib/events-frame.coffee's addEventHandler('overflow'), which as of fx78 does not receive events from OOP frames. --- extension/lib/events-frame.coffee | 4 ++++ extension/lib/markable-elements.coffee | 4 +++- extension/lib/utils.coffee | 17 +++++++++++++++-- extension/lib/viewport.coffee | 3 ++- 4 files changed, 24 insertions(+), 4 deletions(-) diff --git a/extension/lib/events-frame.coffee b/extension/lib/events-frame.coffee index e238613..d3ec3c5 100644 --- a/extension/lib/events-frame.coffee +++ b/extension/lib/events-frame.coffee @@ -110,6 +110,10 @@ class FrameEventManager ) @listen('overflow', (event) => + # XXX(fission): this eventually calls utils::containsDeep(), which if the + # element that caused the event sits in an out-of-process iframe, might + # cause a SecurityError. However, as of Nightly 78, events originating + # from such frames are not raised. target = event.originalTarget @vim.state.scrollableElements.addChecked(target) ) diff --git a/extension/lib/markable-elements.coffee b/extension/lib/markable-elements.coffee index 6b6448d..c765f42 100644 --- a/extension/lib/markable-elements.coffee +++ b/extension/lib/markable-elements.coffee @@ -48,7 +48,9 @@ getMarkableElements = ( ) wrappers.push(wrapper) - for frame in window.frames when frame.frameElement + # Note: with fission's out-of-process iframes, accessing frameElement might + # result in a SecurityError. In this case, squelch it and don't drill down. + for frame in window.frames when (try frame.frameElement) continue unless result = viewportUtils.getFrameViewport( frame.frameElement, viewport ) diff --git a/extension/lib/utils.coffee b/extension/lib/utils.coffee index 0a17828..ac8d74b 100644 --- a/extension/lib/utils.coffee +++ b/extension/lib/utils.coffee @@ -89,7 +89,10 @@ isDevtoolsElement = (element) -> ) isDevtoolsWindow = (window) -> - return window.location?.href in [ + # Note: this function is called for each frame by isDevtoolsElement. When + # called on an out-of-process iframe, accessing .href will fail with + # SecurityError; the `try` around it makes it `undefined` in such a case. + return (try window.location?.href) in [ 'about:devtools-toolbox' 'chrome://devtools/content/framework/toolbox.xul' 'chrome://devtools/content/framework/toolbox.xhtml' # fx72+ @@ -235,11 +238,20 @@ getActiveElement = (window) -> else if activeElement.contentWindow and not (activeElement.localName == 'browser' and activeElement.getAttribute?('contextmenu') == 'contentAreaContextMenu') + # with Fission enabled, the iframe might be located in a different process + # (oop). Then, recursing into it isn't possible (throws SecurityError). + return activeElement unless (try activeElement.contentWindow.document) + return getActiveElement(activeElement.contentWindow) else return activeElement getFocusType = (element) -> switch + when element.tagName in ['FRAME', 'IFRAME'] and + not (try element.contentWindow.document) + # Encountered an out-of-process iframe, which we can't inspect. We fall + # back to insert mode, so any text inputs it may contain are still usable. + 'editable' when isIgnoreModeFocusType(element) 'ignore' when isTypingElement(element) @@ -381,7 +393,8 @@ clearSelectionDeep = (window, {blur = true} = {}) -> # The selection might be `null` in hidden frames. selection = window.getSelection() selection?.removeAllRanges() - for frame in window.frames + # Note: accessing frameElement fails on oop iframes (fission); skip those. + for frame in window.frames when (try frame.frameElement) clearSelectionDeep(frame, {blur}) # Allow parents to re-gain control of text selection. frame.frameElement.blur() if blur diff --git a/extension/lib/viewport.coffee b/extension/lib/viewport.coffee index a57c040..808db27 100644 --- a/extension/lib/viewport.coffee +++ b/extension/lib/viewport.coffee @@ -69,7 +69,8 @@ getAllRangesInsideViewport = (window, viewport, offset = {left: 0, top: 0}) -> } ranges.push({range, rect: adjustedRect}) - for frame in window.frames + # Note: accessing frameElement fails on oop iframes (fission), so we skip them + for frame in window.frames when (try frame.frameElement) {viewport: frameViewport, offset: frameOffset} = getFrameViewport(frame.frameElement, viewport) ? {} continue unless frameViewport -- 2.39.3