Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: remove CDP-specific preferences from defaults for Firefox #11477

Merged
merged 2 commits into from Dec 5, 2023

Conversation

OrKoN
Copy link
Collaborator

@OrKoN OrKoN commented Nov 30, 2023

Closes #11474

@OrKoN OrKoN marked this pull request as draft November 30, 2023 15:24
@whimboo
Copy link
Collaborator

whimboo commented Dec 1, 2023

After a discussion with @OrKoN yesterday and some additional investigation on our side I found the problem that is causing some tests around out-of-process iframes to fail. I filed https://bugzilla.mozilla.org/show_bug.cgi?id=1867667 and we will work on it shortly.

We should wait for it to be fixed on Nightly and then retrigger the tests on this PR.

Copy link
Collaborator

@whimboo whimboo left a comment

Choose a reason for hiding this comment

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

Code-wise this looks all fine. Lets see what happens with test results once we have the contentWindow issue fixed.

@whimboo
Copy link
Collaborator

whimboo commented Dec 4, 2023

3 out of 5 failures are fixed now in the recent version of Firefox Nightly. We took a brief look at the remaining two failing tests and they are related to a click on an element that is within a cross-origin iframe. Hereby the click doesn't happen with the iframe's browsing context reference but the one for the top-level browsing context.

@OrKoN and @sadym-chromium I assume that this is expected? If yes, does it block landing these pref changes?

@OrKoN
Copy link
Collaborator Author

OrKoN commented Dec 5, 2023

@whimboo I believe this behavior is expected from Puppeteer's perspective. It does not work in Firefox only for now or is our expectation not spec-compliant?

@OrKoN OrKoN force-pushed the orkon/fix-firefox-prefs branch 2 times, most recently from bc390c0 to cf32f30 Compare December 5, 2023 09:38
@whimboo
Copy link
Collaborator

whimboo commented Dec 5, 2023

@whimboo I believe this behavior is expected from Puppeteer's perspective. It does not work in Firefox only for now or is our expectation not spec-compliant?

We are having a project early next year to add support for dispatching events from the parent process asynchronously. Until then the event needs to be submitted from the same browsing context.

Maybe we keep the site isolation feature off for now to keep it working, or turn it off by default and file an issue with details for a workaround for now in case cross-origin iframes are involved.

What would be your option?

@OrKoN OrKoN force-pushed the orkon/fix-firefox-prefs branch 3 times, most recently from 3d7489a to 4da9de2 Compare December 5, 2023 10:45
@OrKoN
Copy link
Collaborator Author

OrKoN commented Dec 5, 2023

Let's keep it working for now by turning off the isolation feature.

@OrKoN OrKoN marked this pull request as ready for review December 5, 2023 11:17
@OrKoN OrKoN force-pushed the orkon/fix-firefox-prefs branch 2 times, most recently from 1491a2f to 38636c1 Compare December 5, 2023 15:26
@OrKoN OrKoN enabled auto-merge (squash) December 5, 2023 15:29
@OrKoN OrKoN merged commit f8c9469 into main Dec 5, 2023
56 checks passed
@OrKoN OrKoN deleted the orkon/fix-firefox-prefs branch December 5, 2023 15:40
@release-please release-please bot mentioned this pull request Dec 5, 2023
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.

Don't set CDP-specific custom preferences for Firefox when running with the WebDriver BiDi protocol
3 participants