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

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

Closed
haskida opened this issue Aug 1, 2019 · 4 comments · Fixed by #4913
Closed

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

haskida opened this issue Aug 1, 2019 · 4 comments · Fixed by #4913
Assignees
Labels
pkg/driver This is due to an issue in the packages/driver directory type: regression A bug that didn't appear until a specific Cy version release

Comments

@haskida
Copy link

haskida commented Aug 1, 2019

Current behavior:

Since the merge of #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

@haskida haskida changed the title Cypress incorrectly fires onFocus event when .focus() is called on a hidden element Cypress incorrectly fires onFocus event when .focus() is called on a hidden element starting on v3.3.2 Aug 1, 2019
@cypress-bot cypress-bot bot added the stage: ready for work The issue is reproducible and in scope label Aug 2, 2019
@jennifer-shehane
Copy link
Member

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 7efd9d8

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: 7efd9d8#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.

@jennifer-shehane jennifer-shehane added the type: regression A bug that didn't appear until a specific Cy version release label Aug 2, 2019
@jennifer-shehane jennifer-shehane added the pkg/driver This is due to an issue in the packages/driver directory label Aug 2, 2019
@jennifer-shehane jennifer-shehane changed the title Cypress incorrectly fires onFocus event when .focus() is called on a hidden element starting on v3.3.2 Cypress incorrectly fires onFocus event when .focus() on a hidden element starting on v3.3.2 Aug 2, 2019
@jennifer-shehane jennifer-shehane changed the title Cypress incorrectly fires onFocus event when .focus() on a hidden element starting on v3.3.2 Cypress incorrectly fires onFocus event on a hidden element starting on v3.3.2 Aug 2, 2019
@jennifer-shehane jennifer-shehane changed the title Cypress incorrectly fires onFocus event on a hidden element starting on v3.3.2 nFocus event incorrectly fires on a hidden element starting on v3.3.2 Aug 2, 2019
@jennifer-shehane jennifer-shehane changed the title nFocus event incorrectly fires on a hidden element starting on v3.3.2 onFocus event incorrectly fires on a hidden element starting on v3.3.2 Aug 2, 2019
@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 Aug 2, 2019
@haskida
Copy link
Author

haskida commented Aug 2, 2019

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

@cypress-bot cypress-bot bot added stage: work in progress and removed stage: needs review The PR code is done & tested, needs review labels Aug 2, 2019
@cypress-bot 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 Sep 24, 2019
@jennifer-shehane jennifer-shehane added stage: pending release and removed stage: needs review The PR code is done & tested, needs review labels Oct 1, 2019
@jennifer-shehane
Copy link
Member

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
Copy link
Contributor

cypress-bot bot commented Oct 23, 2019

Released in 3.5.0.

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 type: regression A bug that didn't appear until a specific Cy version release
Projects
None yet
3 participants