fix: elements with opacity: 0 are not visible

[panzarino]

• Closes Opacity is not considered for visibility checks #4474

User facing changelog

BREAKING CHANGE: Elements with opacity: 0 are no longer considered to be visible. This includes elements with an ancestor that has opacity: 0 since a child element can never have a computed opacity greater than that of an ancestor.

Additional details

This is a breaking change since it has the potential to change the behavior of existing tests that rely on elements with opacity: 0 to be visible.

Docs PR: cypress-io/cypress-documentation#3071

PR Tasks

• Have tests been added/updated?
• Has the original issue been tagged with a release in ZenHub?
• Has a PR for user-facing changes been opened in cypress-documentation?
• Have API changes been updated in the type definitions?

[cypress-bot]

Thanks for taking the time to open a PR!

• Create a Draft Pull Request if your PR is not ready for review. Mark the PR as Ready for Review when you're ready for a Cypress team member to review the PR.
• Become familiar with the Code Review Checklist for guidelines on coding standards and what needs to be done before a PR can be merged.

[cypress]

---

Test summary

137 • 0 • 5 • 0

---

Run details

Project	cypress
Status	Passed
Commit	bfe6020
Started	Aug 28, 2020 7:08 AM
Ended	Aug 28, 2020 7:16 AM
Duration	07:29 💡
OS	Linux Debian - 10.2
Browser	Chrome 83

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]

I'd love to have a test case for testing something changing opacity. I feel like the most common use case is someone having an element with opacity: 100, click something which triggers the element to have opacity: 0. So we'd want to make sure that when they assert 'el is not visible' that it waits for opacity to reach 0 and passes. I think this will work from the code that's written, but a test would be nice to confirm.

Is the ignoreOpacity option meant to be user facing? It seems it's just hard coded to be false. Why is it there at all?

Looks good to me otherwise. I think Brian had some notes about the mocha assertions and opacity that he wanted to make sure was covered. Not sure if y'all touched base on that.

[panzarino]

@jennifer-shehane Just added a test for that.

Regarding ignoreOpacity, it is not intended to be user facing and it can be considered hard-coded when the user is making an assertion. From now on, we will always consider opacity: 0 to not be visible when asserting on visibility. However, we still need to consider these elements actionable. Therefore, we have to check all of the other components of visibility, excluding opacity. That's why I changed ensureVisibility to call isHidden directly (with ignoreOpacity: true) rather than using the jquery selector.

Adding this additional parameter seemed to be the easiest and cleanest way of achieving the desired behavior. Outside of isHidden there is no easy way to detect if an element is hidden only because of opacity (doing something like isVisible() || opacity == 0 logically doesn't work). We could do something like moving the opacity check in getReasonIsHidden to the end and checking if the reason is opacity, but I didn't feel like that was a good solution. Plus, we have to check for parent opacity since a child can never have an opacity greater than that of a parent which makes ignoreOpacity the only practical solution. I've added tests that complement my (somewhat convoluted) explanation here.

Selenium has this option, too.

dismissing previous review

[jennifer-shehane]

@panzarino It seems that the sizzle selector calls into the isHidden method, calling it with the element, context (document), and xml. This xml is evaluating to false, which was always setting the ignoreOpacity option to false on these calls.

Since updating the logic to try and accept an object, it's always set to false when using the :hidden selectors, so I don't think this option is being read in correctly.

Put a debugger in isHidden method - see that 3rd arg is always called with false from sizzle: https://github.com/cypress-io/cypress/blob/issue-4474/packages/driver/src/dom/visibility.js#L23:L23

Maybe I'm misunderstanding something about the implementation.

[panzarino]

@jennifer-shehane Looks like jQuery always calls the selector with 3 parameters, the element, a context (which is always the document), a a third xml which is always false. I'm surprised that the second parameter hasn't caused any issues with our implementation yet. I've updated jquery.js to only accept the first element parameter for our overwrite, and this seems to fix the issue.

[thienthu1410]

I just checked on the page https://demoqa.com/automation-practice-form, and I see that: one element has opacity: 0, but it is still visible to my eyes

But Cypress treat it as invisible

Version Cypress: 13.0.0

[NityaQA]

I think its not fixed properly. As same issue I am facing. I tried to assert a checkbox, which is clearly visible to my eye. But cypress is throwing element is not visible as opacity:0

[jennifer-shehane]

@NityaQA Please open a new issue filling out the issue template