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

position: fixed elements having parent with pointer-events: none mistakenly display as not visible / covered by another el #6675

Closed
josh-byster opened this issue Mar 8, 2020 · 8 comments · Fixed by #8095 · May be fixed by Omrisnyk/npm-lockfiles#145 or Omrisnyk/npm-lockfiles#164
Assignees
Labels
pkg/driver This is due to an issue in the packages/driver directory topic: visibility 👁 type: bug

Comments

@josh-byster
Copy link

josh-byster commented Mar 8, 2020

Current behavior:

Hi,
Currently, when testing out the visibility of .spinner for nprogress, it shows as being not visible due to the position being fixed although the spinning circle loader is certainly visible on the page. I'm aware of the complexity of the current algorithm used to determine visibility so I thought this might help give a simple, reproducible example of where this may (if I'm understanding correctly) fail. It says that it's being covered by the html wrapper itself, but the element has a z-index set to 1031.

Desired behavior:

It should be considered visible as it's not being covered by any element in the DOM.

Test code to reproduce

https://github.com/josh-byster/cypress-visibility

There is a visibility.spec.js which has a basic assertion of visibility in there that should show the issue.
If there's any trouble with reproducibility, it can probably be tested directly at the nprogress website to check if the spinner is visible.

Versions

Cypress 4.1.0
Mac OS X Catalina 10.15.3

@jennifer-shehane
Copy link
Member

jennifer-shehane commented Mar 10, 2020

This seems to be due to the parent element of the position: fixed element having the style pointer-events: none, which makes the element what we would call "not actionable", but it seems to be incorrectly also regarding it as "not visible".

I'm not seeing this ever really working correctly. Tried in 3.3.0, 3.4.0, 3.5.0, 4.1.0.

Reproducible Example:

index.html

<html>
<body>
  <div style="pointer-events: none;">
    <div class="spinner" style="position: fixed;"><div/>
  </div>
</body>
</html>

spec.js

it('pointer events on fixed element', () => {
  cy.visit('index.html')
  cy.get('.spinner').should('be.visible')
})

Why

Seems to be a note from brian a long time ago regarding this #1887 (comment):

It's because setting pointer-events: none changes how the browser yields elements via document.elementFromPoint.

In that case the element is "falling through" which triggers Cypress into thinking its being covered up. There's a note / documentation somewhere that talks about pointer-events: none not being handled correctly.

From https://developer.mozilla.org/en-US/docs/Web/API/DocumentOrShadowRoot/elementFromPoint

Elements with pointer-events set to none will be ignored, and the element below it will be returned.

Code that needs fixing

This function is called to determine if a fixed element is being covered by another element, which is essentially calling .elementFromPoint. I'm not exactly sure how to approach fixing completely though.

https://github.com/cypress-io/cypress/blob/update-cypress-cache-list-6404/packages/driver/src/dom/visibility.js#L238:L238

@jennifer-shehane jennifer-shehane changed the title Visibility Error with Loader position: fixed elements having parent with pointer-events: none mistakenly display as not visible / covered by another el Mar 10, 2020
@jennifer-shehane jennifer-shehane added type: bug topic: visibility 👁 pkg/driver This is due to an issue in the packages/driver directory labels Mar 10, 2020
@cypress-bot cypress-bot bot added the stage: ready for work The issue is reproducible and in scope label Mar 10, 2020
@josh-byster
Copy link
Author

Thank you for looking into this further @jennifer-shehane!

@samjetski
Copy link

I've come across a variation on the same problem. In my case it's a tooltip on a modal/overlay, which means the nesting is inverted (pointer-events: none element inside position: fixed container).

index.html

<!DOCTYPE html>
<html>
<head></head>
<body>
<div style="position: fixed;">
    <div id="tooltip" style="pointer-events: none; border: 2px solid maroon;">Tooltip</div>
</div>
</body>
</html>

spec.js

it('pointer events within fixed element', () => {
    cy.visit('index.html');
    cy.get('#tooltip').should('be.visible');
});

One solution that comes to mind is that if the tested element (or any parent) has pointer-events: none then:

  1. Take the fall-through element (which is competing for "most visible") and the tested element
  2. Walk up to the nearest common parent
  3. Compare z-index, DOM order, etc. of elements up the tree to find out which "wins" being on top

Is this kind of thing realistic? I'm not at all familiar with the Cypress internals. One drawback is that we don't know if there are other obscuring elements with pointer-events: none in front, though maybe we just need to live with that.

Also out of curiosity, why does this only happen when there is a parent with position: fixed?

@jennifer-shehane
Copy link
Member

@samjetski The majority of the visibility code can be found here: https://github.com/cypress-io/cypress/blob/develop/packages/driver/src/dom/visibility.js#L20:L20

Doing something like noted is not uncalled for for how we check visibility of elements. It's very specific checks.

@TheDutchCoder
Copy link

I'm wondering why there's no z-index checking in the tests? https://github.com/cypress-io/cypress/blob/715d07c1a9308ad215fe35da2775100a31dbf982/packages/driver/cypress/integration/dom/visibility_spec.ts

There's also no mention of z-index or stacking in the source for visibility which I find suspicious.
I'm not entirely sure what method ends up throwing the error, my best guess would be either elIsNotElementFromPoint or elIsOutOfBoundsOfAncestorsOverflow.

@jennifer-shehane
Copy link
Member

This is still reproducible in Cypress 4.9.0

@panzarino panzarino self-assigned this Jul 27, 2020
@cypress-bot cypress-bot bot added stage: work in progress stage: needs review The PR code is done & tested, needs review and removed stage: ready for work The issue is reproducible and in scope stage: work in progress stage: needs review The PR code is done & tested, needs review labels Jul 27, 2020
@cypress-bot cypress-bot bot added stage: pending release and removed stage: needs review The PR code is done & tested, needs review labels Jul 29, 2020
@cypress-bot
Copy link
Contributor

cypress-bot bot commented Jul 29, 2020

The code for this is done in cypress-io/cypress#8095, but has yet to be released.
We'll update this issue and reference the changelog when it's released.

@cypress-bot
Copy link
Contributor

cypress-bot bot commented Aug 3, 2020

Released in 4.12.0.

This comment thread has been locked. If you are still experiencing this issue after upgrading to
Cypress v4.12.0, please open a new issue.

@cypress-bot cypress-bot bot locked as resolved and limited conversation to collaborators Aug 3, 2020
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.