Allow cy.contains() to opt out of removing whitespace matching #6405

c32hedge · 2020-02-10T22:42:35Z

Current behavior:

Per https://docs.cypress.io/guides/references/migration-guide.html#cy-contains-ignores-invisible-whitespaces, cy.contains now ignores extra whitespace. Apparently #5653 only excluded the <pre> tag from the change, which is not sufficient for modern web applications using arbitrary custom tags.

Desired behavior:

cy.contains should respect an element's white-space css property instead of just looking at whether it's a <pre> tag.

Honestly, I'd like to see the change added in 4.0.0 rolled back, since this is breaking our existing tests that specifically check that whitespace is preserved in some custom elements, and I would argue that the change made was too simplistic to be useful in modern CSS3/HTML5 development.

Alternatively, it would be nice to at least have an option for whether to ignore whitespace, similarly to the option that was added for matchCase.

Test code to reproduce

<!DOCTYPE html>
<html>
  <head>
    <title>White-space example</title>
  </head>
  <body>
    <p id="pre" style="white-space: pre;">this
      field  should preserve all
      whitespace</p>
    <p id="pre-wrap" style="white-space: pre-wrap;">this
      field  should preserve all
      whitespace</p>
  </body>
</html>

describe('css white-space', () => {
  before(() => {
    cy.visit('example.html');
  });

  it('should preserve whitespace in the pre paragraph', () => {
    cy.get('p#pre').contains('this\n      field  should preserve all\n      whitespace');
  });

  it('should preserve whitespace in the pre-wrap paragraph', () => {
    cy.get('p#pre-wrap-1').contains('this\n      field  should preserve all\n      whitespace');
  });
});

Versions

Cypress 4.0.0 and later

The text was updated successfully, but these errors were encountered:

c32hedge · 2020-02-10T22:51:06Z

Ideally, cy.contains should match the expected behavior of all the css white-space values, including differences in whether linebreaks in the source are preserved, and whether linebreaks are inserted to wrap long lines.

I was going to build a more comprehensive set of test cases, but I'm short on time and that would probably be part of fixing this anyway. I did notice that in both 3.8.3 and 4.0.1, attempting to just pass a newline (as in, cy.contains('\n')) gives the error:

cy.contains() cannot be passed an empty string

...which made it difficult to write a comprehensive set of tests around wrapping behavior, in particular asserting that long lines of text in elements with white-space values of normal, pre-wrap, pre-line, and break-spaces contain a newline. Depending on how comprehensive the Cypress team wants the fix for this issue to be, that could be a separate issue.

jennifer-shehane · 2020-02-11T07:44:45Z

I am personally in favor of adding an option for matchWhitespace, as I can just see the requests for exceptions growing in this case. I'm not sure white-space will cover everyone's use case. Having an option to ignore the whitespace matching would be best.

I'm actually not seeing this particular example that you've given passing in 3.8.3. It fails in both 3.8.3 and 4.0.0.

Can you provide an exact example that was working in 3.8.3 and is now failing in 4.0.0? I'd like to understand the use case completely that is causing the breaking changes and this doesn't appear to represent it. Thanks!

<!DOCTYPE html>
<html>
<body>
  <p id="pre" style="white-space: pre;">this
    field should preserve all
    whitespace</p>
  <p id="pre-wrap" style="white-space: pre-wrap;">this
    field should preserve all
    whitespace</p>
</body>
</html>

spec.js

describe('css white-space', () => {
  before(() => {
    cy.visit('index.html');
  });

  it('should preserve whitespace in the pre paragraph', () => {
    cy.get('p#pre').contains('this\n      field  should preserve all\n      whitespace');
  });

  it('should preserve whitespace in the pre-wrap paragraph', () => {
    cy.get('p#pre-wrap').contains('this\n      field  should preserve all\n      whitespace');
  });
});

Exact same error in 3.8.3 and 4.0.0

jennifer-shehane · 2020-02-11T07:59:18Z

Related issue with another point of wanting to opt out of the new ignoring invisible whitespace behavior, this issue matches my point even more of just adding an option to opt-out. #6397

sainthkh · 2020-02-11T08:56:04Z

I created a new PR for matchWhitespaces option and white-space style in #6411. I'm waiting for the feedback.

c32hedge · 2020-02-11T13:28:06Z

Could matchWhitespace=false be the default? (side note--personally matchWhitespace sounds more idiomatic than matchWhitespaces since I'm used to "whitespace" referring to any amount of, well, whitespace as opposed to individual characters).

This would technically be a re-breaking change from 4.0.0 and 4.0.1, but given they were only released a few days ago I wonder if that's ok--it would actually reduce the migration burden for people who haven't upgraded yet, and I don't particularly like the idea of having to clutter every place I include extra whitespace in a string passed to contains with a "yes, I meant to put that whitespace in my string".

Isn't it safer to assume that if someone passes a string with additional whitespace to cy.contains, they passed it for a reason? Cypress test code is in a different context than HTML and, IMO, test writers are likely to be more cognizant of putting whitespace in strings as opposed to in HTML where elements can span multiple lines and are likely in many cases to be wrapped based on editor width rather than by their intended display.

As a side note, there are a couple different interpretation of matching whitespace:

For an HTML element with a white-space value of normal, to me, ignoring whitespace would mean both of the following to work:

<p>a  normal element   with some whitespace</p>

cy.get("p").contains("a  normal element   with some whitespace");
cy.get("p").contains("a normal element with some whitespace");

But I would expect the first example to also work if the additional whitespace was present in the displayed HTML. Stripping whitespace from a string I specified, by default, seems to me to violate the principle of least surprise.

It seems to me you could potentially get the best of both worlds by, instead of having to have a matchWhitespace option, using the following logic:

Attempt to find the string passed to cy.contains without stripping any whitespace characters
If it matches, return true
If it doesn't match, try again after stripping whitespace
If it matches, return true

At that point, I would want the matchWhitespace option to prevent the second check from happening, but this would be independent of looking at the white-space value, specific tags, etc.

jennifer-shehane · 2020-02-11T14:59:11Z

@c32hedge No, we will not be releasing another breaking change for this which would require 5.0.0 release.

We believe the default should be to ignore extra whitespaces which is why we merged this change in to begin with. The majority of cases were just extra newlines or spaces that are not effectually rendered for the user to see. Having an option to ignore this, will solve your use case.

c32hedge · 2020-02-11T16:05:10Z

I'm not sure we're on the same wavelength 😁.

Referring back to #92, it looks like the original desire was for something like this to work:

HTML

<h1>hello
world</h1>

Test

cy.contains("hello world")

I didn't see any mention there of expecting the reverse (ignoring whitespace that was explicitly passed to cy.contains to be ignored.

Basically I'm arguing that:

Yes, the majority of cases where the HTML contains additional whitespace not rendered by the browser, testers would want to ignore that whitespace.
But I expect the majority of cases where the cypress test code contains additional whitespace, the author put it there for a reason, so having to pass an option to explicitly make Cypress pay attention to it feels "wrong" and may actually mask poor test code in the cases where ignoring the whitespace happens to make the tests pass.

A couple examples to help illustrate:

I would consider the expected breaking change in 4.0.0 to be things like the example below, where someone explicitly added whitespace to cy.contains to work around the issue described in #92:

HTML

<h1>hello
world</h1>

Test

cy.contains("hello\nworld")

That would be a good thing to have start failing in 4.0.0, since it highlights code that was depending on the actual whitespace in the source file instead of what the browser actually renders and the user sees. The fix would be to remove the newline from the cy.contains call.

However, the behavior introduced so that the following that used to work now fails, I would consider a regression, rather than a breaking change that can't be rolled back until another major version:

HTML

<h1 style="white-space: pre-wrap">hello
world</h1>

Test

cy.contains("hello\nworld")

Sorry, I think the wording of my previous comment wasn't very clear. Also, I'm working on making a cypress-test-tiny branch that will reproduce the examples I shared in the original issue description, and I'm including the examples from this comment.

The first four tests pass in Cypress 3.8.3, but fail in 4.0.1 The last two tests fail in Cypress 3.8.3, but pass in 4.0.1 The desire is that all tests pass

c32hedge · 2020-02-11T16:29:44Z

Added example repo.

sainthkh · 2020-02-12T01:47:49Z

@c32hedge After reading your comment, it seems to me that what you want is not whether the default value of matchWhitespace is true or false.

You want Cypress to pass the tests which check the elements with white-space: pre-wrap without specifying matchWhitespace option.

If I'm reading correctly, it's implemented in #6411. (Click here to see the tests.)

But it seems that we cannot support pre-line in 4.0.0 for now because it's another breaking change and it is inconsistent between browsers. And this can take longer because it's a bit related with #6384.

EDIT: Confirmed that one thing is not covered:

cy.contains('this field should preserve all whitespace').should('not.exist') // should pass but fails.

c32hedge · 2020-02-12T16:03:23Z

@sainthkh After reading the code in #6411 and some of the surrounding code, yes, this looks like what I want. I realized I was mistaken on how Cypress was handling the text I passed to cy.contains -- the code only calls normalizeWhitespaces on the elem, never on the text that was passed to cy.contains, so indeed adding additional logic to respect css's white-space should do the trick.

c32hedge · 2020-02-12T16:53:56Z

@sainthkh Actually, I'm wondering if this is even simpler.

I noticed the following line in the existing code for normalizeWhitespaces:

let testText = elem.textContent || elem.innerText || $.text(elem)

Does everything "just work", without all the new logic for looking at the white-space property, if you swap the precedent to the following?

let testText = elem.innerText || elem.textContent || $.text(elem)

(see https://developer.mozilla.org/en-US/docs/Web/API/Node/textContent#Differences_from_innerText)

In addition, you might want to add a test for something like the example HTML on the Mozilla page describing innerText:

<p id="source">
  <style>#source { color: red; }</style>
Take a look at<br>how this text<br>is interpreted
       below.
  <span style="display:none">HIDDEN TEXT</span>
</p>

The following test fails in Cypress 4.0.1:

  describe('this tests what happens if you have style elements inline', () => {
    it('should not contain the style code text', () => {
      cy.get('#source').contains('color: red').should("not.exist");
    });
  });

Based on the documentation linked above though, it looks like innerText should give us the text as rendered by the browser without having to do the postprocessing to determine whether to strip whitespace, and would also fix issues like this one with style code getting mixed into the string.

codeheroics · 2020-04-07T17:01:35Z

We believe the default should be to ignore extra whitespaces which is why we merged this change in to begin with. The majority of cases were just extra newlines or spaces that are not effectually rendered for the user to see. Having an option to ignore this, will solve your use case.

Maybe a new main method be added to Cypress to better express the difference between the "old" contains and the "new" contains?

I'm facing a codebase with a few breakages (and currently no solution) following this change, because many of our texts come from wordings in files which are concatenated / rearranged together, which makes us end up with some strings like "multiple {variable} spaces". This did not use to matter, because the same variables were used both in the HTML and in cy.contains(), but the new behaviour makes this break everywhere.

It seems that the old .contains() method allowed to match something that approximated what was present in the HTML (and thus easier to assert for) while the new method tests for the visual presence of the space. I'd argue that having to test for the visual presence of the space is rarer, but perhaps a htmlContains method could restore the old behaviour? It seems more explicit than adding a matchWhitespace flag.

I find it weird that this breaking changes, which was specifically announced in the migration guide as "cy.contains() ignores invisible whitespaces", turns out to have an impact that is quite the opposite, and to really be that cy.contains tests that visually, the number of white-spaces displayed match the string given to it. It also seems to me that this should be the edge case, because as far as I know, it should only matters when white-space has a value of pre, pre-line or pre-wrap.

Narretz · 2020-04-29T08:40:36Z

Is there an ETA for the whitespace flag, or any otherchange to this behavior @sainthkh?

This change in v4 was very confusing to me. I've made a little repo to show my use case:
https://github.com/Narretz/cypress-whitespace-test
(Note: you have to run the server with npm run serve before running cypress):

Basically, I've used contains() to match text distributed over separate elements. Before v4, I could basically count the whitespace between elements to get the string I needed to assert against.
With v4+, for the string needed, it was really trial and error, because not all whitespace I assumed was ignored, was actually ignored (I've added a few cases in the spec file).
I would really like to see this behavior reverted, configurable, or reworked.

cypress-app-bot · 2023-05-17T22:21:08Z

This issue has not had any activity in 180 days. Cypress evolves quickly and the reported behavior should be tested on the latest version of Cypress to verify the behavior is still occurring. It will be closed in 14 days if no updates are provided.

DRiewaldt · 2023-05-18T17:48:25Z

Bad bot, it still behaves this way and a flag to ignore white space would be helpful unless there is a new command I should be using instead of contains.

Some items might have a trailing whitespace, and the easiest way to handle this would be to simply ignore white space and match the text characters only.

Any update?

nagash77 · 2023-05-18T18:06:26Z

Hi @DRiewaldt , I will update this feature accordingly with the correct labels to make sure this issue is not picked up as stale in the future. I will also forward this along to our product team for review. Thank you for your response.

danielSoellerAtBearP · 2023-11-23T15:16:26Z

I would love to see, an Update here. The Idea of, having a flag or any other Option to turn this of would be great.

c32hedge mentioned this issue Feb 10, 2020

Cypress will convert double space to a single space. #6397

Closed

sainthkh self-assigned this Feb 11, 2020

sainthkh added type: regression A bug that didn't appear until a specific Cy version release v4.0.0 🐛 Issue present since 4.0.0 pkg/driver This is due to an issue in the packages/driver directory labels Feb 11, 2020

jennifer-shehane added type: unexpected behavior User expected result, but got another and removed type: regression A bug that didn't appear until a specific Cy version release labels Feb 11, 2020

jennifer-shehane added the stage: ready for work The issue is reproducible and in scope label Feb 11, 2020

jennifer-shehane changed the title ~~cy.contains new behavior in 4.0.0 doesn't take css white-space property into account~~ Allow cy.contains() to opt out of removing whitespace matching Feb 11, 2020

sainthkh mentioned this issue Feb 11, 2020

Add matchWhitespaces option to cy.contains. #6411

Closed

4 tasks

cypress-bot bot added stage: work in progress and removed stage: ready for work The issue is reproducible and in scope labels Feb 11, 2020

cypress-bot bot added stage: needs review The PR code is done & tested, needs review stage: work in progress and removed stage: work in progress stage: needs review The PR code is done & tested, needs review labels Feb 11, 2020

jennifer-shehane mentioned this issue Feb 24, 2020

[4.0.2] cy.within().contains() regression, element no longer found #6519

Closed

codeheroics mentioned this issue Apr 7, 2020

Cypress 4.0.0 broke the matching of non-standard spaces #6959

Closed

spankyed mentioned this issue Aug 2, 2022

should('have.text', ...) should ignore leading and trailing whitespace #3887

Open

marktnoonan mentioned this issue Feb 24, 2023

Selectors in cy.get() and cy.contains() cannot find invalid table HTML elements in some situations #25893

Open

cypress-app-bot added the stale no activity on this issue for a long period label May 17, 2023

nagash77 added type: feature New feature that does not currently exist E2E Issue related to end-to-end testing and removed stale no activity on this issue for a long period labels May 18, 2023

nagash77 unassigned sainthkh May 18, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow cy.contains() to opt out of removing whitespace matching #6405

Allow cy.contains() to opt out of removing whitespace matching #6405

c32hedge commented Feb 10, 2020

c32hedge commented Feb 10, 2020 •

edited

Loading

jennifer-shehane commented Feb 11, 2020

jennifer-shehane commented Feb 11, 2020

sainthkh commented Feb 11, 2020

c32hedge commented Feb 11, 2020

jennifer-shehane commented Feb 11, 2020

c32hedge commented Feb 11, 2020

c32hedge commented Feb 11, 2020 •

edited

Loading

sainthkh commented Feb 12, 2020 •

edited

Loading

c32hedge commented Feb 12, 2020

c32hedge commented Feb 12, 2020 •

edited

Loading

codeheroics commented Apr 7, 2020

Narretz commented Apr 29, 2020

cypress-app-bot commented May 17, 2023

DRiewaldt commented May 18, 2023

nagash77 commented May 18, 2023

danielSoellerAtBearP commented Nov 23, 2023

Allow cy.contains() to opt out of removing whitespace matching #6405

Allow cy.contains() to opt out of removing whitespace matching #6405

Comments

c32hedge commented Feb 10, 2020

Current behavior:

Desired behavior:

Test code to reproduce

Versions

c32hedge commented Feb 10, 2020 • edited Loading

jennifer-shehane commented Feb 11, 2020

jennifer-shehane commented Feb 11, 2020

sainthkh commented Feb 11, 2020

c32hedge commented Feb 11, 2020

jennifer-shehane commented Feb 11, 2020

c32hedge commented Feb 11, 2020

c32hedge commented Feb 11, 2020 • edited Loading

sainthkh commented Feb 12, 2020 • edited Loading

c32hedge commented Feb 12, 2020

c32hedge commented Feb 12, 2020 • edited Loading

codeheroics commented Apr 7, 2020

Narretz commented Apr 29, 2020

cypress-app-bot commented May 17, 2023

DRiewaldt commented May 18, 2023

nagash77 commented May 18, 2023

danielSoellerAtBearP commented Nov 23, 2023

c32hedge commented Feb 10, 2020 •

edited

Loading

c32hedge commented Feb 11, 2020 •

edited

Loading

sainthkh commented Feb 12, 2020 •

edited

Loading

c32hedge commented Feb 12, 2020 •

edited

Loading