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: The content of <script> and <style> inside <body> should be ignored with cy.contains. #17477

Merged
merged 3 commits into from
Aug 4, 2021

Conversation

sainthkh
Copy link
Contributor

@sainthkh sainthkh commented Jul 26, 2021

User facing changelog

  • cy.contains() no longer yields the <body> element when it matches the content of <script> or <style> tags.

Additional details

  • Why was this change necessary? => In most cases, users don't want to search the string inside <script> or <style> tag inside the <body>
  • What is affected by this change? => Some tests that used the workaround might fail with this change.
  • Any implementation details to explain? => When testing body tag, <script> and <style> tags inside it will be removed

How has the user experience changed?

N/A

Notes

  • I'm a bit late to the party. It should be targetted to Cypress 9.0.
  • I think we don't need option like includeScriptContent, includeStyleContent, do we?

PR Tasks

  • Have tests been added/updated?

@cypress-bot
Copy link
Contributor

cypress-bot bot commented Jul 26, 2021

Thanks for taking the time to open a PR!

cy.contains('some-script-content').should('not.match', 'script')
cy.contains('some-script-content').should('not.exist')
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This code was added at #5079. It had to be written in this way because cy.contains() returns <body> tag at that time.

It is changed, because it simply fails.

@sainthkh sainthkh marked this pull request as ready for review July 26, 2021 05:40
@sainthkh sainthkh requested a review from a team as a code owner July 26, 2021 05:40
@sainthkh sainthkh requested review from chrisbreiding and jennifer-shehane and removed request for a team July 26, 2021 05:40
@jennifer-shehane jennifer-shehane added the type: breaking change Requires a new major release version label Jul 26, 2021
@jennifer-shehane jennifer-shehane changed the base branch from develop to 9.0-release July 27, 2021 15:42
@jennifer-shehane
Copy link
Member

Updated base branch to 9.0-release branch

@jennifer-shehane
Copy link
Member

Thanks @sainthkh, we're prioritizing reviewing other work for now for the immediate release but do plan to review this when we can.

- moves removing style and script elements into own function, since it's not related to normalizing whitespace
- adds nested style and script elements to html fixture. this fails the test with the previous implementation, which only removed non-nested ones
- uses jquery for removing elements, which will remove nested style and script elements as well as non-nested ones
@frabcus
Copy link

frabcus commented Nov 15, 2021

Thanks everyone for making this change, it is very much appreciated!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: breaking change Requires a new major release version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

cy.contains('xxx').should('not.exist') try to expect <body> not to exist in the DOM
4 participants