Skip to content

Commit

Permalink
Remove the arbitrary timeout in the "must check that first text field…
Browse files Browse the repository at this point in the history
… 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.
  • Loading branch information
Snuffleupagus committed Dec 19, 2020
1 parent 54f45dc commit 6f40f4e
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 2 deletions.
5 changes: 4 additions & 1 deletion test/integration/scripting_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
);
Expand Down
24 changes: 23 additions & 1 deletion web/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
}
});
},

/**
Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit 6f40f4e

Please sign in to comment.