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: Avoid looking up remote jquery instance if subject is not an element #8300

Merged
merged 1 commit into from Aug 17, 2020

Conversation

chrisbreiding
Copy link
Contributor

@chrisbreiding chrisbreiding commented Aug 17, 2020

TR-244

Should fix this flaky failure.

The error originates from this cy.readFile().then call. We try to look up the remote jQuery instance from even though the subject is not a dom element. This causes it to sometimes fail with a cross-origin error in Firefox.

This prevents attempting to look up the remote jQuery instance in cases where the subject is not a dom element.

PR Tasks

  • Have tests been added/updated?

@cypress-bot
Copy link
Contributor

cypress-bot bot commented Aug 17, 2020

Thanks for taking the time to open a PR!

@cypress
Copy link

cypress bot commented Aug 17, 2020



Test summary

8120 0 108 3


Run details

Project cypress
Status Passed
Commit ca48410
Started Aug 17, 2020 1:51 PM
Ended Aug 17, 2020 1:57 PM
Duration 05:59 💡
OS Linux Debian - 10.2
Browser Multiple

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

@jennifer-shehane
Copy link
Member

It's weird this has never failed before since this file hasn't been touched in over a year. But I guess we don't normally run docs in Firefox either.

@chrisbreiding
Copy link
Contributor Author

It's weird this has never failed before since this file hasn't been touched in over a year. But I guess we don't normally run docs in Firefox either.

Is it new that we run the docs against Firefox? I am wondering what changed that caused this to start failing.

@chrisbreiding
Copy link
Contributor Author

I've tried adding a test for this, but it seems impossible to reproduce reliably. At the very least, this won't create any new issues since the logic hasn't changed. It just ensures we don't try to access state('window').$ when we don't need to by moving it after the isElement() check.

@chrisbreiding chrisbreiding merged commit 093f3e7 into develop Aug 17, 2020
@jennifer-shehane
Copy link
Member

Yah, maybe it is new that we run docs in Firefox in develop.

@chrisbreiding chrisbreiding deleted the tr-244-firefox-cross-origin-failure branch April 5, 2022 18:29
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.

None yet

3 participants