From 02b597315b121cee62d9f156862bef4322403533 Mon Sep 17 00:00:00 2001 From: ROpdebee <15186467+ROpdebee@users.noreply.github.com> Date: Tue, 21 Jun 2022 21:44:49 +0200 Subject: [PATCH] fix(split links): run in iframes in Chrome Chrome does not fire the window load event on iframe windows, Firefox doesn't fire the DOMContentLoaded event. Instead, we should just listen for the iframe load event itself, rather than load events on the contained window or document. --- src/lib/util/dom.ts | 16 ++++-- src/mb_multi_external_links/index.ts | 15 ++---- tests/unit/lib/util/dom.test.ts | 81 +++++++++------------------- 3 files changed, 44 insertions(+), 68 deletions(-) diff --git a/src/lib/util/dom.ts b/src/lib/util/dom.ts index 18ea7c6fc..61415ad03 100644 --- a/src/lib/util/dom.ts +++ b/src/lib/util/dom.ts @@ -47,11 +47,21 @@ export function onDocumentLoaded(listener: () => void): void { } } -export function onWindowLoaded(listener: () => void, windowInstance: Window = window): void { - if (windowInstance.document.readyState === 'complete') { +export function onWindowLoaded(listener: () => void): void { + if (window.document.readyState === 'complete') { listener(); } else { - windowInstance.addEventListener('load', listener); + window.addEventListener('load', listener); + } +} + +export function onAddEntityDialogLoaded(dialog: HTMLIFrameElement, listener: () => void): void { + // iframe could already have finished loading. We can detect this as the + // absence of the loading div. + if (qsMaybe('.content-loading', dialog.parentElement!) === null) { + listener(); + } else { + dialog.addEventListener('load', () => { listener(); }); } } diff --git a/src/mb_multi_external_links/index.ts b/src/mb_multi_external_links/index.ts index 153092333..043b45433 100644 --- a/src/mb_multi_external_links/index.ts +++ b/src/mb_multi_external_links/index.ts @@ -7,7 +7,7 @@ import { LOGGER } from '@lib/logging/logger'; import { assertDefined } from '@lib/util/assert'; import { logFailure, retryTimes } from '@lib/util/async'; import { createPersistentCheckbox } from '@lib/util/checkboxes'; -import { onWindowLoaded, qsa, qsMaybe, setInputValue } from '@lib/util/dom'; +import { onAddEntityDialogLoaded, qsa, qsMaybe, setInputValue } from '@lib/util/dom'; import DEBUG_MODE from 'consts:debug-mode'; import USERSCRIPT_ID from 'consts:userscript-id'; @@ -160,15 +160,10 @@ async function run(windowInstance: Window): Promise { function onIframeAdded(iframe: HTMLIFrameElement): void { LOGGER.debug(`Initialising on iframe ${iframe.src}`); - const iframeWindow = iframe.contentWindow; - if (!iframeWindow) return; - - // Cannot use onDocumentLoaded even if we make it accept a custom document - // since iframe contentDocument doesn't fire the DOMContentLoaded event in - // Firefox. - onWindowLoaded(() => { - logFailure(run(iframeWindow)); - }, iframeWindow); + + onAddEntityDialogLoaded(iframe, () => { + logFailure(run(iframe.contentWindow!)); + }); } // Observe for additions of embedded entity creation dialogs and run the link diff --git a/tests/unit/lib/util/dom.test.ts b/tests/unit/lib/util/dom.test.ts index 25a4484b9..cb741d4e3 100644 --- a/tests/unit/lib/util/dom.test.ts +++ b/tests/unit/lib/util/dom.test.ts @@ -131,61 +131,32 @@ describe('callback on window loaded', () => { cb.mockReset(); }); - function runTests(windowInstance?: Window): void { - it('does not fire if the window is not loaded', () => { - jest.spyOn(windowInstance?.document ?? document, 'readyState', 'get').mockReturnValue('interactive'); - const cb = jest.fn(); - onWindowLoaded(cb, windowInstance); - - expect(cb).not.toHaveBeenCalled(); - }); - - it('fires if the window was already loaded', () => { - jest.spyOn(windowInstance?.document ?? document, 'readyState', 'get').mockReturnValue('complete'); - const cb = jest.fn(); - onWindowLoaded(cb, windowInstance); - - expect(cb).toHaveBeenCalledOnce(); - }); - - it('fires after the window was loaded', () => { - jest.spyOn(windowInstance?.document ?? document, 'readyState', 'get').mockReturnValue('interactive'); - const cb = jest.fn(); - onWindowLoaded(cb, windowInstance); - - expect(cb).not.toHaveBeenCalled(); - - (windowInstance ?? window).dispatchEvent(new Event('load')); - - expect(cb).toHaveBeenCalledOnce(); - }); - } - - describe('with main window', () => { - // eslint-disable-next-line jest/require-hook - runTests(); - }); - - describe('with custom window', () => { - const customWindow = { - eventListener: null as (() => void) | null, - document: { - get readyState(): string { - return 'uninitialized'; - }, - }, - addEventListener(evt: string, listener: () => void): void { - if (evt === 'load') this.eventListener = listener; - }, - dispatchEvent(evt: Event): void { - if (evt.type === 'load') { - this.eventListener?.(); - } - }, - }; - - // eslint-disable-next-line jest/require-hook - runTests(customWindow as unknown as Window); + it('does not fire if the window is not loaded', () => { + jest.spyOn(document, 'readyState', 'get').mockReturnValue('interactive'); + const cb = jest.fn(); + onWindowLoaded(cb); + + expect(cb).not.toHaveBeenCalled(); + }); + + it('fires if the window was already loaded', () => { + jest.spyOn(document, 'readyState', 'get').mockReturnValue('complete'); + const cb = jest.fn(); + onWindowLoaded(cb); + + expect(cb).toHaveBeenCalledOnce(); + }); + + it('fires after the window was loaded', () => { + jest.spyOn(document, 'readyState', 'get').mockReturnValue('interactive'); + const cb = jest.fn(); + onWindowLoaded(cb); + + expect(cb).not.toHaveBeenCalled(); + + window.dispatchEvent(new Event('load')); + + expect(cb).toHaveBeenCalledOnce(); }); });