cypress-io / cypress

Fast, easy and reliable testing for anything that runs in a browser.
https://cypress.io
MIT License
46.42k stars 3.14k forks source link

onFocus event incorrectly fires on a hidden element starting on v3.3.2 #4898

Closed haskida closed 4 years ago

haskida commented 4 years ago

Current behavior:

Since the merge of https://github.com/cypress-io/cypress/pull/2982 in v3.3.2, the hidden search bar on the site https://www.unibet.com/betting/sports/home is always opened on cy.visit() of the page Screenshot 2019-08-01 at 3 06 41 PM

This does not happen when the site is visited manually without Cypress

Desired behavior:

The search bar remains hidden, same as if the page is visited manually.

Steps to reproduce: (app code and test code)

  1. Navigate to the page https://www.unibet.com/betting/sports/home with cy.visit()

Versions

Issue started with Cypress 3.3.2

jennifer-shehane commented 4 years ago

Thanks for giving us so much direction on tracking this down and also providing a reproducible example! It helps a lot.

To reproduce

it('focus should not fire on input.d647', () => {
  cy.visit('https://www.unibet.com/betting/sports/home')
})

I was able to narrow this down to version 3.3.2 of Cypress, although I did not specifically narrow down the issue to this commit https://github.com/cypress-io/cypress/commit/7efd9d8ab0db34424b2854a2ee3d1f1bf463ffee

So, upon adding a listener to the focus event, I do not see the listener be caught in the live site at all.

Screen Shot 2019-08-02 at 11 59 44 AM

When running in Cypress I see the focus event caught initially on the a#CybotCookiebotDialogBodyButtonAccept.CybotCookiebotDialogBodyButton element and then subsequently on the input.d647 element in this method. https://github.com/cypress-io/cypress/blob/develop/packages/driver/src/cy/focused.coffee#L150

Screen Shot 2019-08-02 at 12 02 08 PM

The notes on this method say

## normally programmatic focus calls cause "primed" focus/blur
## events if the window is not in focus
## so we fire fake events to act as if the window
## is always in focus

So, what's unique about this input? Maybe the fact that it has display: none computed CSS property, not by a CSS style on itself, but on one of it's parents like.

<div style='display: none'>
  <div>
    <input>
  <div>
</div>

It looks like we do a pretty simple check on if an element is focusable here: https://github.com/cypress-io/cypress/commit/7efd9d8ab0db34424b2854a2ee3d1f1bf463ffee#diff-22a86df1bb6a99b0a980cb1cf50fef23R28

Just from a brief reading of the spec, it looks like elements that are hidden should not be considered focusable. BUT, I didn't read in depth into this.

Partial Focusable definition

Elements that have their tabindex focus flag set, that are not actually disabled, that are not expressly inert, and that are either being rendered or being used as relevant canvas fallback content. https://html.spec.whatwg.org/multipage/interaction.html#data-model

Being rendered definition

An element is being rendered if it has any associated CSS layout boxes, SVG layout boxes, or some equivalent in other styling languages. NOTE: Just being off-screen does not mean the element is not being rendered. The presence of the hidden attribute normally means the element is not being rendered, though this might be overridden by the style sheets. https://html.spec.whatwg.org/multipage/rendering.html#being-rendered

I think the hidden definition includes elements with display: none, but that became a rabbit hole I will leave up to @Bkucera to narrow down.

haskida commented 4 years ago

Awesome! thanks for the quick response and PR already in place :)

jennifer-shehane commented 4 years ago

The code for this is done, but this has yet to be released. We'll update this issue and reference the changelog when it's released.

cypress-bot[bot] commented 4 years ago

Released in 3.5.0.