-
Notifications
You must be signed in to change notification settings - Fork 3.4k
chore: bump firefox version we test against to 141 #32078
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
Conversation
cypress
|
||||||||||||||||||||||||||||||||||||||||
| Project |
cypress
|
| Branch Review |
bump-firefox-141
|
| Run status |
|
| Run duration | 20m 03s |
| Commit |
|
| Committer | Cacie Prins |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
|
|
0
|
|
|
15
|
|
|
1230
|
|
|
0
|
|
|
32179
|
| View all changes introduced in this branch ↗︎ | |
UI Coverage
45.83%
|
|
|---|---|
|
|
190
|
|
|
165
|
Accessibility
97.95%
|
|
|---|---|
|
|
3 critical
8 serious
2 moderate
2 minor
|
|
|
107
|
b013f30 to
e97be81
Compare
| // Firefox bidi returns "unspecified" for sameSite; | ||
| // webkit & firefox < 135 return "no_restriction" for sameSite; | ||
| // other browsers do not return sameSite at all | ||
| const sameSite = ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the comment is helpful, but do we need the conditional logic as we really test the version of firefox rolling forward?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We still test against FF 134 in CI
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's my fault I keep thinking Cypress 15 🥴
| return ( | ||
| browser.family === 'firefox' && ( | ||
| Number(browser.majorVersion) < 135 || | ||
| (Number(browser.majorVersion) < 141 && !!process.env.FORCE_FIREFOX_CDP) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would say it might be worth mentioning FORCE_FIREFOX_CDP doesn't work in Firefox 141 plus in a console warning but I am not convinced this option is used enough to warrant the extra work
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's implied by the new warning message
| // Firefox bidi returns "unspecified" for sameSite; | ||
| // webkit & firefox < 135 return "no_restriction" for sameSite; | ||
| // other browsers do not return sameSite at all | ||
| const sameSite = ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's my fault I keep thinking Cypress 15 🥴
|
Released in This comment thread has been locked. If you are still experiencing this issue after upgrading to |
Additional details
141 Released today
Slack thread for details on Firefox 140 change to cookies
Steps to test
How has the user experience changed?
PR Tasks
cypress-documentation?type definitions?