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

Inline, zero-dimension element: erroneous visibility determination #6183

Closed
badeball opened this issue Jan 17, 2020 · 8 comments · Fixed by #8130 · May be fixed by Omrisnyk/npm-lockfiles#145
Closed

Inline, zero-dimension element: erroneous visibility determination #6183

badeball opened this issue Jan 17, 2020 · 8 comments · Fixed by #8130 · May be fixed by Omrisnyk/npm-lockfiles#145
Assignees
Labels
pkg/driver This is due to an issue in the packages/driver directory topic: visibility 👁 type: bug

Comments

@badeball
Copy link
Contributor

badeball commented Jan 17, 2020

Current behavior:

This example illustrates an element with offsetWidth and offsetHeight of zero, but is still interactable in actuality.

For reference, Firefox does not report these values to be zero, which might be of interest in regards to expanding browser support (ie. maybe not good to rely on it as it's not consistent across).

Desired behavior:

For the element not to be reported as invisible.

Test code to reproduce

https://github.com/badeball/cypress-reproducible-issues/tree/master/zero-dimension-hover

Versions

Cypress v3.8.2, Arch Linux, Chrome 79.

@jennifer-shehane
Copy link
Member

jennifer-shehane commented Jan 22, 2020

I have confirmed this is a bug in 3.8.2. The <span> is considered not interactable, but when hovering as a real user, the span is able to respond to mouseover events.

index.html

<html>
<body>
  <p>
    <span onmouseover="window.hover.style.display='block'" onmouseout="window.hover.style.display='none'">
      <label style="display: block">Hover me!</label>
    </span>
  </p>
  <p id="hover" style="display: none">
    Hovering!
  </p>
</body>
</html>

spec.js

it("should work", () => {
  cy.visit("index.html")
  cy.get("span").trigger("mouseover")
})

@cypress-bot cypress-bot bot added the stage: ready for work The issue is reproducible and in scope label Jan 22, 2020
@jennifer-shehane jennifer-shehane added type: bug topic: visibility 👁 stage: ready for work The issue is reproducible and in scope pkg/driver This is due to an issue in the packages/driver directory and removed stage: ready for work The issue is reproducible and in scope labels Jan 22, 2020
@jennifer-shehane
Copy link
Member

This is still reproducible in Cypress 4.9.0.

@sainthkh
Copy link
Contributor

The simplest solution is to add display:block style to the element and check the size of it. Because the size of inline blocks are 0.

But the problem is that it becomes harder to determine the visibility when width:0 and height:0 is set. In those cases, the offsetWidth is 0 even when display:block while the text is visible and hover is working.

Below is the case.

<html>
<body>
  <p>
    <span id="target" onmouseover="window.hover.style.display='block'" onmouseout="window.hover.style.display='none'" style="display:block;width:0">
      <label>Hover me!</label>
    </span>
  </p>
  <p id="hover" style="display: none">
    Hovering!
  </p>
</body>
</html>

@panzarino panzarino self-assigned this Jul 29, 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 labels Jul 30, 2020
@panzarino
Copy link
Contributor

I am not fully convinced that we should actually fix this issue.

See the following recommendation from W3: https://www.w3.org/TR/2009/CR-CSS2-20090908/visuren.html#anonymous-block-level

When an inline box contains a block box, the inline box (and its inline ancestors within the same line box) are broken around the block. The line boxes before the break and after the break are enclosed in anonymous boxes, and the block box becomes a sibling of those anonymous boxes.

This description makes it appear as if the block box should not be considered to be part of the inline box. Therefore, the inline element is correctly being reported as not visible.

In addition, since the span takes up no space in the DOM, it's impossible that a user can actually interact with it. What you're really looking to emulate here is the user hovering over the label, and then the onmouseover event is sent. I think the proper test would be to actually do cy.get("label").trigger("mouseover"), which generates the proper result:

Screen Shot 2020-07-30 at 12 17 19 PM

@jennifer-shehane
Copy link
Member

Oof, yah, I get the technicalities of this not technically being able to take action, but the actionability portion of this is not very intuitive to someone just trying to test their app.

I think the original example is really counterintuitive to workaround as is (without this fix). You set the event handler literally on that span DOM element - then trigger that on that DOM element. It's hard to expect the user to drive down to whatever DOM elements are nested inside when they know they want the mouseover to trigger on that span in the end. All they know is - "I physically mouse over this in my app and the onmouseover is triggered".

The visibility check is also a bit weird since what exactly about this span is visible? So, I kind of get that maybe not passing.......

@panzarino
Copy link
Contributor

@jennifer-shehane I feel like it makes most sense that Cypress acts just as a user would act in the browser. The user cannot hover over the span in the DOM, even though the HTML makes it appear that a user would be able to. If the user can't interact, Cypress shouldn't be able to either even if its regarding a technicality in the DOM structure. Note, this is all regarding how Chromium renders.

I do think that we should implement this fix. Firefox actually will wrap the span around the label in this case, which causes the spec in this issue to work just fine. For sake of reducing flake between browsers and such, I think we should implement the fix for consistency reasons.

@cypress-bot cypress-bot bot added stage: pending release and removed stage: needs review The PR code is done & tested, needs review labels Aug 17, 2020
@cypress-bot
Copy link
Contributor

cypress-bot bot commented Aug 17, 2020

The code for this is done in cypress-io/cypress#8130, 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 20, 2020

Released in 5.0.0.

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

@cypress-bot cypress-bot bot locked as resolved and limited conversation to collaborators Aug 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
pkg/driver This is due to an issue in the packages/driver directory topic: visibility 👁 type: bug
Projects
None yet
4 participants