Add Firefox support#1359
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Test summary
Run details
View run in Cypress Dashboard ➡️ This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard |
edbb977 to
38338d8
Compare
|
@bkucera I got new from Twitter that this work picked up again, amazing to see it 🤩! Developers I talk to love Cypress and keep apologizing to me in conversations for not testing Firefox. Since I work on Developer Experiences team at Firefox I be would be happy to chat and to help with any questions, bugs or feature requests. |
|
Is this still a priority @chrisbreiding @bkucera? last commit is pretty old... |
They seem to have been working on it in bursts, but even still September 24th hardly feels "pretty old".
|
|
@joshsleeper @andrewholsted 2 weeks is 6 months in javascript-years, but yes I have been busy with 3.5 release. This PR is very close |
| path: string | ||
| isHeaded: boolean | ||
| isHeadless: boolean | ||
| family: string |
There was a problem hiding this comment.
don't we already define this interface inside of packages/launcher? or somewhere else?
There was a problem hiding this comment.
probably because there is no clean way to share code between packages and this - a lot of types are duplicated because of that. it would be nice to revisit how we do types to untangle this mess.
brian-mann
left a comment
There was a problem hiding this comment.
overall pretty damn good.
i didn't leave comments on a lot of uncommented code since i know it's still WIP
| { | ||
| "globals": { | ||
| "Cypress": true | ||
| } | ||
| } |
There was a problem hiding this comment.
this is code smell needing to pull in globals - i get the challenge - you want DOM utils to be stateless, and avoid DI or instantiation but there's probably a middle ground here we can facilitate
| @@ -36,6 +36,15 @@ const attachMouseDblclickListeners = attachListeners(['dblclick']) | |||
| const attachContextmenuListeners = attachListeners(['contextmenu']) | |||
There was a problem hiding this comment.
why are there so many lines changed in this file... ?
| .should('have.focus') | ||
|
|
||
| if (Cypress.browser.family === 'firefox') { | ||
| cy.wait(0).get('@selectionchange').should('be.called') |
There was a problem hiding this comment.
whys this necessary?
all of these conditionals need a comment else its completely unclear and opaque why its being done
This comment has been minimized.
This comment has been minimized.
|
❤️ the commit velocity in this pull request. I saw that you are working around a few issues already. If you hit any blockers in Firefox, let me know and I can help solve them. |
* firefox headless * update errors * fix video recording in ff headless * fix thos types
… issue-1096-firefox-support
|
@JenniferFuBook We're working overtime to get this out, as can be seen by our commits in this branch. So, please be patient, fixing a few remaining bugs. 🙏 |
Co-authored-by: Zach Bloomquist <github@chary.us>
… issue-1096-firefox-support
User facing changelog
Cypress now support running tests in Firefox.
Additional details
Tasks
windowobjectsfix not setting selected broswer withnot a bug, only sets default browser when setting custom browserbrowserlaunch arge2e/video_spec)chromeWebSecurity: falsewhen running firefox (desktop gui, terminal reporter)How has the user experience changed?
beta flag for initial release:
chromeWebSecuritywarning (desktop-gui):before:browser:launchwarning (desktop-gui):warnings in
runmode:forced GC warning
TODO: add screenshot of forced GC warning
PR Tasks
cypress-documentation? Add docs for Firefox support cypress-documentation#420type definitions?cypress.schema.json?Additional testing @bahmutov
later can add