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 shadow dom visibility hang #7893

Merged
merged 2 commits into from Jul 7, 2020

Conversation

chrisbreiding
Copy link
Contributor

User facing changelog

  • Fixed an issue where checking for visibility on a shadow dom host element would hang if the host element was the foremost element and had an ancestor with fixed position.

Additional details

More information in this comment on the original issue. The fix itself is preventing an infinite loop and is pretty straightforward.

I also refactored the code to use recursion instead of while loops, since a while loop will hang the browser, making it difficult to debug. If a recursive function infinitely recurses, it usually causes a Maximum stack... error pretty quickly without hanging the browser and the stack for the error points directly to the offending function.

PR Tasks

  • Have tests been added/updated?
  • Has the original issue been tagged with a release in ZenHub?
  • N/A Has a PR for user-facing changes been opened in cypress-documentation?
  • N/A Have API changes been updated in the type definitions?
  • N/A Have new configuration options been added to the cypress.schema.json?

makes diagnosing bugs in terminal condition easier because it causes a 'Maximum call stack size exceeded' error pointing to the offending function instead of hanging with no indication where it's happening
@cypress-bot
Copy link
Contributor

cypress-bot bot commented Jul 6, 2020

Thanks for taking the time to open a PR!

@cypress
Copy link

cypress bot commented Jul 6, 2020



Test summary

7850 0 129 0


Run details

Project cypress
Status Passed
Commit df04977
Started Jul 6, 2020 1:57 PM
Ended Jul 6, 2020 2:04 PM
Duration 06:07 💡
OS Linux Debian - 10.1
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

Copy link
Member

@jennifer-shehane jennifer-shehane left a comment

Choose a reason for hiding this comment

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

Verified that this fixes both of the originally provided examples.

Code looks a lot clearer. 👍

const isWithinShadowRoot = (node: HTMLElement) => {
return node.getRootNode().toString() === '[object ShadowRoot]'
return isShadowRoot(node.getRootNode())
Copy link
Contributor

Choose a reason for hiding this comment

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

👍 nice, I know this is something James was hoping to fix in his original PR.

@chrisbreiding chrisbreiding deleted the issue-7794-shadow-dom-visibility-hang 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.

Bug: Cypress hangs when detecting element visibility with Shadow DOM enabled
3 participants