Skip to content

Commit 592093f

Browse files
committed
feat(click): start wiring auto-waiting click in firefox
1 parent 1bf5b61 commit 592093f

File tree

5 files changed

+51
-15
lines changed

5 files changed

+51
-15
lines changed

package.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
"main": "index.js",
1010
"playwright": {
1111
"chromium_revision": "747023",
12-
"firefox_revision": "1032",
12+
"firefox_revision": "1035",
1313
"webkit_revision": "1168"
1414
},
1515
"scripts": {

src/firefox/ffPage.ts

+11-2
Original file line numberDiff line numberDiff line change
@@ -60,10 +60,11 @@ export class FFPage implements PageDelegate {
6060
helper.addEventListener(this._session, 'Page.frameDetached', this._onFrameDetached.bind(this)),
6161
helper.addEventListener(this._session, 'Page.navigationAborted', this._onNavigationAborted.bind(this)),
6262
helper.addEventListener(this._session, 'Page.navigationCommitted', this._onNavigationCommitted.bind(this)),
63-
helper.addEventListener(this._session, 'Page.navigationStarted', this._onNavigationStarted.bind(this)),
63+
helper.addEventListener(this._session, 'Page.navigationStarted', event => this._onNavigationStarted(event.frameId)),
6464
helper.addEventListener(this._session, 'Page.sameDocumentNavigation', this._onSameDocumentNavigation.bind(this)),
6565
helper.addEventListener(this._session, 'Runtime.executionContextCreated', this._onExecutionContextCreated.bind(this)),
6666
helper.addEventListener(this._session, 'Runtime.executionContextDestroyed', this._onExecutionContextDestroyed.bind(this)),
67+
helper.addEventListener(this._session, 'Page.linkClicked', event => this._onLinkClicked(event.frameId, event.ack)),
6768
helper.addEventListener(this._session, 'Page.uncaughtError', this._onUncaughtError.bind(this)),
6869
helper.addEventListener(this._session, 'Runtime.console', this._onConsole.bind(this)),
6970
helper.addEventListener(this._session, 'Page.dialogOpened', this._onDialogOpened.bind(this)),
@@ -116,7 +117,15 @@ export class FFPage implements PageDelegate {
116117
}
117118
}
118119

119-
_onNavigationStarted() {
120+
_onLinkClicked(frameId: string, ack: boolean) {
121+
if (ack)
122+
this._page._frameManager.frameDidRequestNavigation();
123+
else
124+
this._page._frameManager.frameWillRequestNavigation();
125+
}
126+
127+
_onNavigationStarted(frameId: string) {
128+
this._page._frameManager.frameRequestedNavigation(frameId);
120129
}
121130

122131
_onNavigationAborted(params: Protocol.Page.navigationAbortedPayload) {

src/frames.ts

+21-7
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,9 @@ export class FrameManager {
5959
private _mainFrame: Frame;
6060
readonly _lifecycleWatchers = new Set<() => void>();
6161
readonly _consoleMessageTags = new Map<string, ConsoleTagHandler>();
62-
private _navigationRequestCollectors = new Set<Set<string>>();
62+
private _inflightNavigationFrames: Set<string> | undefined;
63+
private _requestNavigationPromise: Promise<void> | undefined;
64+
private _requestNavigationPromiseCallback: (() => void) | undefined;
6365

6466
constructor(page: Page) {
6567
this._page = page;
@@ -110,28 +112,40 @@ export class FrameManager {
110112

111113
async waitForNavigationsCreatedBy<T>(action: () => Promise<T>): Promise<T> {
112114
const frameIds = new Set<string>();
113-
this._navigationRequestCollectors.add(frameIds);
115+
this._inflightNavigationFrames = frameIds;
114116
try {
115117
const result = await action();
118+
if (this._requestNavigationPromise)
119+
await this._requestNavigationPromise;
116120
if (!frameIds.size)
117121
return result;
118122
const frames = Array.from(frameIds.values()).map(frameId => this._frames.get(frameId));
119123
await Promise.all(frames.map(frame => frame!.waitForNavigation({ waitUntil: []}))).catch(e => {});
120124
await new Promise(platform.makeWaitForNextTask());
121125
return result;
122126
} finally {
123-
this._navigationRequestCollectors.delete(frameIds);
127+
this._inflightNavigationFrames = undefined;
124128
}
125129
}
126130

131+
frameWillRequestNavigation() {
132+
this._requestNavigationPromise = new Promise(f => this._requestNavigationPromiseCallback = f);
133+
}
134+
135+
frameDidRequestNavigation() {
136+
if (this._requestNavigationPromiseCallback)
137+
this._requestNavigationPromiseCallback();
138+
this._requestNavigationPromiseCallback = undefined;
139+
}
140+
127141
frameRequestedNavigation(frameId: string) {
128-
for (const frameIds of this._navigationRequestCollectors)
129-
frameIds.add(frameId);
142+
if (this._inflightNavigationFrames)
143+
this._inflightNavigationFrames.add(frameId);
130144
}
131145

132146
_cancelFrameRequestedNavigation(frameId: string) {
133-
for (const frameIds of this._navigationRequestCollectors)
134-
frameIds.delete(frameId);
147+
if (this._inflightNavigationFrames)
148+
this._inflightNavigationFrames.delete(frameId);
135149
}
136150

137151
frameCommittedNewDocumentNavigation(frameId: string, url: string, name: string, documentId: string, initial: boolean) {

test/browsercontext.spec.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -344,7 +344,7 @@ module.exports.describe = function({testRunner, expect, playwright, CHROMIUM, FF
344344
expect(error.message).toBe('Function "baz" has been already registered in one of the pages');
345345
await context.close();
346346
});
347-
it.skip(FFOX)('should be callable from-inside addInitScript', async({browser, server}) => {
347+
it.fail(FFOX)('should be callable from-inside addInitScript', async({browser, server}) => {
348348
const context = await browser.newContext();
349349
let args = [];
350350
await context.exposeFunction('woof', function(arg) {

test/navigation.spec.js

+17-4
Original file line numberDiff line numberDiff line change
@@ -561,7 +561,7 @@ module.exports.describe = function({testRunner, expect, playwright, MAC, WIN, FF
561561
return page.setContent(`<script src='networkidle.js'></script>`, { waitUntil: 'networkidle2' });
562562
}, true);
563563
});
564-
it.skip(FFOX)('should wait for networkidle0 in setContent with request from previous navigation', async({page, server}) => {
564+
it.fail(FFOX)('should wait for networkidle0 in setContent with request from previous navigation', async({page, server}) => {
565565
// TODO: in Firefox window.stop() does not cancel outstanding requests, and we also lack 'init' lifecycle,
566566
// therefore we don't clear inflight requests at the right time.
567567
await page.goto(server.EMPTY_PAGE);
@@ -571,7 +571,7 @@ module.exports.describe = function({testRunner, expect, playwright, MAC, WIN, FF
571571
return page.setContent(`<script src='networkidle.js'></script>`, { waitUntil: 'networkidle0' });
572572
}, true);
573573
});
574-
it.skip(FFOX)('should wait for networkidle2 in setContent with request from previous navigation', async({page, server}) => {
574+
it.fail(FFOX)('should wait for networkidle2 in setContent with request from previous navigation', async({page, server}) => {
575575
// TODO: in Firefox window.stop() does not cancel outstanding requests, and we also lack 'init' lifecycle,
576576
// therefore we don't clear inflight requests at the right time.
577577
await page.goto(server.EMPTY_PAGE);
@@ -800,7 +800,7 @@ module.exports.describe = function({testRunner, expect, playwright, MAC, WIN, FF
800800
});
801801
});
802802

803-
describe.fail(FFOX)('Page.automaticWaiting', () => {
803+
describe('Page.automaticWaiting', () => {
804804
it('clicking anchor should await navigation', async({page, server}) => {
805805
const messages = [];
806806
server.setRoute('/empty.html', async (req, res) => {
@@ -881,7 +881,20 @@ module.exports.describe = function({testRunner, expect, playwright, MAC, WIN, FF
881881
]);
882882
expect(messages.join('|')).toBe('route|waitForNavigation|evaluate');
883883
});
884-
it('should await navigating specified target', async({page, server, httpsServer}) => {
884+
it('evaluating reload should await navigation', async({page, server}) => {
885+
const messages = [];
886+
await page.goto(server.EMPTY_PAGE);
887+
server.setRoute('/empty.html', async (req, res) => {
888+
messages.push('route');
889+
res.end('done');
890+
});
891+
await Promise.all([
892+
page.evaluate(`window.location.reload()`).then(() => messages.push('evaluate')),
893+
page.waitForNavigation({ waitUntil: [] }).then(() => messages.push('waitForNavigation')),
894+
]);
895+
expect(messages.join('|')).toBe('route|waitForNavigation|evaluate');
896+
});
897+
it('should await navigating specified target', async({page, server}) => {
885898
const messages = [];
886899
server.setRoute('/empty.html', async (req, res) => {
887900
messages.push('route');

0 commit comments

Comments
 (0)