From 6f40f4e7c2c46fc51b320d014294e9eebcbb18f8 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Fri, 18 Dec 2020 23:12:52 +0100 Subject: [PATCH] Remove the arbitrary timeout in the "must check that first text field has focus" integration-test (PR 12702 follow-up) It seems that the timeout is way too short in practice, since this new integration-test failed *intermittently* already in PR 12702 (which is where the test was added). The ideal solution here would be to simply await an event, dispatched by the viewer, however that unfortunately doesn't appear to be supported by Puppeteer. Instead, the solution implemented here is to add a new method in `PDFViewerApplication` which Puppeteer can query to check if the scripting/sandbox has been fully initialized. --- test/integration/scripting_spec.js | 5 ++++- web/app.js | 24 +++++++++++++++++++++++- 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/test/integration/scripting_spec.js b/test/integration/scripting_spec.js index bde715249..43fc949d1 100644 --- a/test/integration/scripting_spec.js +++ b/test/integration/scripting_spec.js @@ -30,9 +30,12 @@ describe("Interaction", () => { it("must check that first text field has focus", async () => { await Promise.all( pages.map(async ([browserName, page]) => { + await page.waitForFunction( + "window.PDFViewerApplication.scriptingReady === true" + ); + // The document has an open action in order to give // the focus to 401R. - await page.waitForTimeout(1000); const id = await page.evaluate( () => window.document.activeElement.id ); diff --git a/web/app.js b/web/app.js index 4623b3632..67f558778 100644 --- a/web/app.js +++ b/web/app.js @@ -1480,7 +1480,12 @@ const PDFViewerApplication = { // of the sandbox and removal of the event listeners at document closing. const internalEvents = new Map(), domEvents = new Map(); - this._scriptingInstance = { scripting, internalEvents, domEvents }; + this._scriptingInstance = { + scripting, + ready: false, + internalEvents, + domEvents, + }; if (!this.documentInfo) { // It should be *extremely* rare for metadata to not have been resolved @@ -1612,6 +1617,15 @@ const PDFViewerApplication = { id: "doc", name: "Open", }); + + // Used together with the integration-tests, see the `scriptingReady` + // getter, to enable awaiting full initialization of the scripting/sandbox. + // (Defer this slightly, to make absolutely sure that everything is done.) + Promise.resolve().then(() => { + if (this._scriptingInstance) { + this._scriptingInstance.ready = true; + } + }); }, /** @@ -2242,6 +2256,14 @@ const PDFViewerApplication = { this._wheelUnusedTicks -= wholeTicks; return wholeTicks; }, + + /** + * Used together with the integration-tests, to enable awaiting full + * initialization of the scripting/sandbox. + */ + get scriptingReady() { + return this._scriptingInstance?.ready || false; + }, }; let validateFileURL;