Skip to content

Conversation

yury-s
Copy link
Member

@yury-s yury-s commented Oct 2, 2025

Reference #37277

@yury-s yury-s requested review from dgozman and pavelfeldman October 2, 2025 23:16
@yury-s
Copy link
Member Author

yury-s commented Oct 2, 2025

cc @whimboo

This comment has been minimized.

This comment has been minimized.

Copy link
Contributor

github-actions bot commented Oct 3, 2025

Test results for "tests 1"

2 failed
❌ [playwright-test] › ui-mode-test-network-tab.spec.ts:204 › should not preserve selection across test runs @macos-latest-node18-1
❌ [playwright-test] › reporter-html.spec.ts:2999 › created › execSync doesnt produce a second stdout attachment @macos-latest-node18-2

5 flaky ⚠️ [firefox-library] › library/inspector/cli-codegen-1.spec.ts:1079 › cli codegen › should not throw csp directive violation errors `@firefox-ubuntu-22.04-node18`
⚠️ [firefox-page] › page/page-event-request.spec.ts:182 › should return response body when Cross-Origin-Opener-Policy is set `@firefox-ubuntu-22.04-node18`
⚠️ [firefox-page] › page/page-wait-for-function.spec.ts:104 › should work with strict CSP policy `@firefox-ubuntu-22.04-node18`
⚠️ [playwright-test] › ui-mode-trace.spec.ts:397 › should work behind reverse proxy `@macos-latest-node18-1`
⚠️ [installation tests] › validate-dependencies.spec.ts:20 › should validate dependencies correctly if skipped during install `@package-installations-ubuntu-latest`

46884 passed, 811 skipped


Merge workflow run.

allowsThirdParty: [async ({ browserName }, run) => {
if (browserName === 'firefox' || browserName as any === '_bidiFirefox')
allowsThirdParty: [async ({ browserName, channel }, run) => {
if (browserName === 'firefox' || channel?.startsWith('moz-firefox'))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (browserName === 'firefox' || channel?.startsWith('moz-firefox'))
if (browserName === 'firefox')

it('should not fire events for favicon or favicon redirects', async ({ context, page, server, browserName, headless }) => {
it.skip(headless && browserName !== 'firefox' && browserName as any !== '_bidiFirefox', 'headless browsers, except firefox, do not request favicons');
it('should not fire events for favicon or favicon redirects', async ({ context, page, server, browserName, headless, channel }) => {
it.skip(headless && browserName !== 'firefox' && !channel?.startsWith('moz-firefox'), 'headless browsers, except firefox, do not request favicons');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
it.skip(headless && browserName !== 'firefox' && !channel?.startsWith('moz-firefox'), 'headless browsers, except firefox, do not request favicons');
it.skip(headless && browserName !== 'firefox', 'headless browsers, except firefox, do not request favicons');

it('should load svg favicon with prefer-color-scheme', async ({ page, server, browserName, headless, asset }) => {
it.skip(headless && browserName !== 'firefox' && browserName as any !== '_bidiFirefox', 'headless browsers, except firefox, do not request favicons');
it('should load svg favicon with prefer-color-scheme', async ({ page, server, browserName, headless, asset, channel }) => {
it.skip(headless && browserName !== 'firefox' && !channel?.startsWith('moz-firefox'), 'headless browsers, except firefox, do not request favicons');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
it.skip(headless && browserName !== 'firefox' && !channel?.startsWith('moz-firefox'), 'headless browsers, except firefox, do not request favicons');
it.skip(headless && browserName !== 'firefox', 'headless browsers, except firefox, do not request favicons');

it('should filter favicon and favicon redirects', async ({ server, browserName, headless, asset, contextFactory }, testInfo) => {
it.skip(headless && browserName !== 'firefox' && browserName as any !== '_bidiFirefox', 'headless browsers, except firefox, do not request favicons');
it('should filter favicon and favicon redirects', async ({ server, browserName, headless, asset, contextFactory, channel }, testInfo) => {
it.skip(headless && browserName !== 'firefox' && !channel?.startsWith('moz-firefox'), 'headless browsers, except firefox, do not request favicons');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
it.skip(headless && browserName !== 'firefox' && !channel?.startsWith('moz-firefox'), 'headless browsers, except firefox, do not request favicons');
it.skip(headless && browserName !== 'firefox', 'headless browsers, except firefox, do not request favicons');

expect((await respPromise).message).toContain(kTargetClosedErrorMessage);
// All headers are the same as "provisional" headers in Firefox.
if (browserName === 'firefox' || browserName as any === '_bidiFirefox')
if (browserName === 'firefox' || channel?.startsWith('moz-firefox'))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (browserName === 'firefox' || channel?.startsWith('moz-firefox'))
if (browserName === 'firefox')


// Third part for Firefox is the last one and encodes engine and browser versions.
if (browserName === 'firefox' || browserName as any === '_bidiFirefox') {
if (browserName === 'firefox' || channel?.startsWith('moz-firefox')) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (browserName === 'firefox' || channel?.startsWith('moz-firefox')) {
if (browserName === 'firefox') {

if (browserName === 'webkit' || browserName as any === '_bidiFirefox')
if (browserName === 'webkit' || channel?.startsWith('moz-firefox'))
expect(text).toEqual('Error: Exception');
if (browserName === 'firefox')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (browserName === 'firefox')
else if (browserName === 'firefox')

if (browserName === 'webkit')
expect(error.message).toContain('TypeError');
if (browserName === 'firefox' || browserName === '_bidiFirefox')
if (browserName === 'firefox' || channel?.startsWith('moz-firefox'))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (browserName === 'firefox' || channel?.startsWith('moz-firefox'))
if (browserName === 'firefox')

@yury-s
Copy link
Member Author

yury-s commented Oct 3, 2025

I'd prefer to leave redundant channel?.startsWith('moz-firefox') conditions for documentation purposes and in case we want to use different browser name again. We can always clean it up later, but for now it disambiguates wether the behavior is expected in bidi.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants