diff --git a/packages/puppeteer-core/src/bidi/Frame.ts b/packages/puppeteer-core/src/bidi/Frame.ts index 9b20a13503e..44bec2687ee 100644 --- a/packages/puppeteer-core/src/bidi/Frame.ts +++ b/packages/puppeteer-core/src/bidi/Frame.ts @@ -4,7 +4,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -import type * as Bidi from 'chromium-bidi/lib/cjs/protocol/protocol.js'; +import * as Bidi from 'chromium-bidi/lib/cjs/protocol/protocol.js'; import type {Observable} from '../../third_party/rxjs/rxjs.js'; import { @@ -265,7 +265,14 @@ export class BidiFrame extends Frame { ): Promise { const [response] = await Promise.all([ this.waitForNavigation(options), - this.browsingContext.navigate(url), + this.browsingContext.navigate( + url, + // Some implementations currently only report errors when the + // readiness=interactive. This also ensures that old frames have been + // removed. + // Related: https://bugzilla.mozilla.org/show_bug.cgi?id=1846601 + Bidi.BrowsingContext.ReadinessState.Interactive + ), ]).catch( rewriteNavigationError( url, diff --git a/packages/puppeteer-core/src/bidi/core/Session.ts b/packages/puppeteer-core/src/bidi/core/Session.ts index dd90f8190c2..ffd39769e70 100644 --- a/packages/puppeteer-core/src/bidi/core/Session.ts +++ b/packages/puppeteer-core/src/bidi/core/Session.ts @@ -105,6 +105,19 @@ export class Session browserEmitter.once('closed', ({reason}) => { this.dispose(reason); }); + + // TODO: Currently, some implementations do not emit navigationStarted event + // for fragment navigations (as per spec) and some do. This could emits a + // synthetic navigationStarted to work around this inconsistency. + const seen = new WeakSet(); + this.on('browsingContext.fragmentNavigated', info => { + if (seen.has(info)) { + return; + } + seen.add(info); + this.emit('browsingContext.navigationStarted', info); + this.emit('browsingContext.fragmentNavigated', info); + }); } // keep-sorted start block=yes diff --git a/test/TestExpectations.json b/test/TestExpectations.json index 61ccbe9ee2d..1d93ed5fb98 100644 --- a/test/TestExpectations.json +++ b/test/TestExpectations.json @@ -822,13 +822,6 @@ "parameters": ["webDriverBiDi"], "expectations": ["FAIL"] }, - { - "testIdPattern": "[navigation.spec] navigation Page.goto should fail when navigating to bad SSL", - "platforms": ["darwin", "linux", "win32"], - "parameters": ["firefox"], - "expectations": ["SKIP"], - "comment": "https://github.com/w3c/webdriver-bidi/issues/657" - }, { "testIdPattern": "[navigation.spec] navigation Page.goto should send referer", "platforms": ["darwin", "linux", "win32"], @@ -2226,8 +2219,7 @@ "testIdPattern": "[frame.spec] Frame specs Frame Management should detach child frames on navigation", "platforms": ["darwin", "linux", "win32"], "parameters": ["firefox", "webDriverBiDi"], - "expectations": ["FAIL"], - "comment": "https://github.com/w3c/webdriver-bidi/issues/659" + "expectations": ["PASS"] }, { "testIdPattern": "[frame.spec] Frame specs Frame Management should report different frame instance when frame re-attaches", @@ -2269,8 +2261,7 @@ "testIdPattern": "[frame.spec] Frame specs Frame Management should send \"framenavigated\" when navigating on anchor URLs", "platforms": ["darwin", "linux", "win32"], "parameters": ["firefox", "webDriverBiDi"], - "expectations": ["SKIP"], - "comment": "https://github.com/w3c/webdriver-bidi/issues/657" + "expectations": ["PASS"] }, { "testIdPattern": "[frame.spec] Frame specs Frame Management should send events when frames are manipulated dynamically", @@ -2288,8 +2279,7 @@ "testIdPattern": "[frame.spec] Frame specs Frame Management should support framesets", "platforms": ["darwin", "linux", "win32"], "parameters": ["firefox", "webDriverBiDi"], - "expectations": ["FAIL"], - "comment": "https://github.com/w3c/webdriver-bidi/issues/659" + "expectations": ["PASS"] }, { "testIdPattern": "[frame.spec] Frame specs Frame Management should support lazy frames", @@ -3100,29 +3090,37 @@ "testIdPattern": "[navigation.spec] navigation Page.goBack should work with HistoryAPI", "platforms": ["darwin", "linux", "win32"], "parameters": ["firefox", "webDriverBiDi"], - "expectations": ["SKIP"], - "comment": "browsingContext.navigationStarted event not emitted for fragment navigation" + "expectations": ["PASS"] }, { "testIdPattern": "[navigation.spec] navigation Page.goto should fail when main resources failed to load", "platforms": ["darwin", "linux", "win32"], "parameters": ["firefox", "webDriverBiDi"], - "expectations": ["SKIP"], - "comment": "https://github.com/w3c/webdriver-bidi/issues/657" + "expectations": ["PASS"] }, { "testIdPattern": "[navigation.spec] navigation Page.goto should fail when navigating and show the url at the error message", "platforms": ["darwin", "linux", "win32"], "parameters": ["firefox", "webDriverBiDi"], - "expectations": ["SKIP"], - "comment": "https://github.com/w3c/webdriver-bidi/issues/657" + "expectations": ["PASS"] + }, + { + "testIdPattern": "[navigation.spec] navigation Page.goto should fail when navigating to bad SSL", + "platforms": ["darwin", "linux", "win32"], + "parameters": ["firefox", "webDriverBiDi"], + "expectations": ["PASS"] + }, + { + "testIdPattern": "[navigation.spec] navigation Page.goto should fail when navigating to bad SSL", + "platforms": ["darwin", "linux", "win32"], + "parameters": ["cdp", "firefox"], + "expectations": ["FAIL"] }, { "testIdPattern": "[navigation.spec] navigation Page.goto should fail when navigating to bad SSL after redirects", "platforms": ["darwin", "linux", "win32"], "parameters": ["firefox", "webDriverBiDi"], - "expectations": ["SKIP"], - "comment": "https://github.com/w3c/webdriver-bidi/issues/657" + "expectations": ["PASS"] }, { "testIdPattern": "[navigation.spec] navigation Page.goto should fail when server returns 204", @@ -3134,8 +3132,7 @@ "testIdPattern": "[navigation.spec] navigation Page.goto should fail when server returns 204", "platforms": ["darwin", "linux", "win32"], "parameters": ["firefox", "webDriverBiDi"], - "expectations": ["SKIP"], - "comment": "https://github.com/w3c/webdriver-bidi/issues/657" + "expectations": ["PASS"] }, { "testIdPattern": "[navigation.spec] navigation Page.goto should navigate to dataURL and fire dataURL requests", @@ -3233,6 +3230,13 @@ "parameters": ["cdp", "firefox"], "expectations": ["FAIL"] }, + { + "testIdPattern": "[navigation.spec] navigation Page.goto should work when page calls history API in beforeunload", + "platforms": ["darwin", "linux", "win32"], + "parameters": ["firefox", "webDriverBiDi"], + "expectations": ["FAIL"], + "comment": "History navigation is breaking the Puppeteer expecation about navigation." + }, { "testIdPattern": "[navigation.spec] navigation Page.goto should work with anchor navigation", "platforms": ["darwin", "linux", "win32"], @@ -3243,8 +3247,7 @@ "testIdPattern": "[navigation.spec] navigation Page.goto should work with anchor navigation", "platforms": ["darwin", "linux", "win32"], "parameters": ["firefox", "webDriverBiDi"], - "expectations": ["SKIP"], - "comment": "browsingContext.navigationStarted event not emitted for fragment navigation" + "expectations": ["PASS"] }, { "testIdPattern": "[navigation.spec] navigation Page.goto should work with subframes return 204", @@ -3286,8 +3289,7 @@ "testIdPattern": "[navigation.spec] navigation Page.waitForNavigation should work with DOM history.back()/history.forward()", "platforms": ["darwin", "linux", "win32"], "parameters": ["firefox", "webDriverBiDi"], - "expectations": ["SKIP"], - "comment": "browsingContext.navigationStarted event not emitted for fragment navigation" + "expectations": ["PASS"] }, { "testIdPattern": "[navigation.spec] navigation Page.waitForNavigation should work with history.pushState()", @@ -3299,8 +3301,7 @@ "testIdPattern": "[navigation.spec] navigation Page.waitForNavigation should work with history.pushState()", "platforms": ["darwin", "linux", "win32"], "parameters": ["firefox", "webDriverBiDi"], - "expectations": ["SKIP"], - "comment": "browsingContext.navigationStarted event not emitted for fragment navigation" + "expectations": ["PASS"] }, { "testIdPattern": "[navigation.spec] navigation Page.waitForNavigation should work with history.replaceState()", @@ -3542,7 +3543,8 @@ "testIdPattern": "[network.spec] network Response.fromCache should work", "platforms": ["darwin", "linux", "win32"], "parameters": ["firefox", "webDriverBiDi"], - "expectations": ["FAIL"] + "expectations": ["FAIL"], + "comment": "Needs investigation, it looks like a bug in Puppeteer" }, { "testIdPattern": "[network.spec] network Response.fromCache should work", @@ -3681,8 +3683,7 @@ "testIdPattern": "[oopif.spec] OOPIF should support frames within OOP iframes", "platforms": ["darwin", "linux", "win32"], "parameters": ["firefox", "webDriverBiDi"], - "expectations": ["SKIP"], - "comment": "Fetch error" + "expectations": ["PASS"] }, { "testIdPattern": "[oopif.spec] OOPIF should support lazy OOP frames", @@ -3750,7 +3751,7 @@ "platforms": ["darwin", "linux", "win32"], "parameters": ["firefox", "webDriverBiDi"], "expectations": ["SKIP"], - "comment": "Fetch error" + "comment": "Chrome-specific test (uses DNS mapping); does not work with Firefox." }, { "testIdPattern": "[page.spec] Page Page.addScriptTag should throw when added with content to the CSP page", diff --git a/test/src/navigation.spec.ts b/test/src/navigation.spec.ts index 1f3a51f58af..dd59c983497 100644 --- a/test/src/navigation.spec.ts +++ b/test/src/navigation.spec.ts @@ -154,10 +154,10 @@ describe('navigation', function () { }); const EXPECTED_SSL_CERT_MESSAGE_REGEX = - /net::ERR_CERT_INVALID|net::ERR_CERT_AUTHORITY_INVALID/; + /net::ERR_CERT_INVALID|net::ERR_CERT_AUTHORITY_INVALID|MOZILLA_PKIX_ERROR_SELF_SIGNED_CERT|SSL_ERROR_UNKNOWN/; it('should fail when navigating to bad SSL', async () => { - const {page, httpsServer, isChrome} = await getTestState(); + const {page, httpsServer} = await getTestState(); // Make sure that network events do not emit 'undefined'. // @see https://crbug.com/750469 @@ -176,18 +176,14 @@ describe('navigation', function () { await page.goto(httpsServer.EMPTY_PAGE).catch(error_ => { return (error = error_); }); - if (isChrome) { - expect(error.message).toMatch(EXPECTED_SSL_CERT_MESSAGE_REGEX); - } else { - expect(error.message).toContain('SSL_ERROR_UNKNOWN'); - } + expect(error.message).toMatch(EXPECTED_SSL_CERT_MESSAGE_REGEX); expect(requests).toHaveLength(2); expect(requests[0]).toBe('request'); expect(requests[1]).toBe('requestfailed'); }); it('should fail when navigating to bad SSL after redirects', async () => { - const {page, server, httpsServer, isChrome} = await getTestState(); + const {page, server, httpsServer} = await getTestState(); server.setRedirect('/redirect/1.html', '/redirect/2.html'); server.setRedirect('/redirect/2.html', '/empty.html'); @@ -195,17 +191,10 @@ describe('navigation', function () { await page.goto(httpsServer.PREFIX + '/redirect/1.html').catch(error_ => { return (error = error_); }); - if (isChrome) { - expect(error.message).toMatch(EXPECTED_SSL_CERT_MESSAGE_REGEX); - } else { - expect(error.message).atLeastOneToContain([ - 'MOZILLA_PKIX_ERROR_SELF_SIGNED_CERT', // Firefox WebDriver BiDi. - 'SSL_ERROR_UNKNOWN ', // Others. - ]); - } + expect(error.message).toMatch(EXPECTED_SSL_CERT_MESSAGE_REGEX); }); it('should fail when main resources failed to load', async () => { - const {page, isChrome} = await getTestState(); + const {page} = await getTestState(); let error!: Error; await page @@ -213,11 +202,9 @@ describe('navigation', function () { .catch(error_ => { return (error = error_); }); - if (isChrome) { - expect(error.message).toContain('net::ERR_CONNECTION_REFUSED'); - } else { - expect(error.message).toContain('NS_ERROR_CONNECTION_REFUSED'); - } + expect(error.message).toMatch( + /net::ERR_CONNECTION_REFUSED|NS_ERROR_CONNECTION_REFUSED/ + ); }); it('should fail when exceeding maximum navigation timeout', async () => { const {page, server} = await getTestState(); diff --git a/tools/sort-test-expectations.mjs b/tools/sort-test-expectations.mjs index d1c8588d8a2..e826fcd58d7 100644 --- a/tools/sort-test-expectations.mjs +++ b/tools/sort-test-expectations.mjs @@ -43,6 +43,10 @@ testExpectations.forEach(item => { item.parameters.sort(); item.expectations.sort(); item.platforms.sort(); + // Delete comments for PASS expectations. They are likely outdated. + if (item.expectations.length === 1 && item.expectations[0] === 'PASS') { + delete item.comment; + } }); if (process.argv.includes('--lint')) {