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

You cannot programmatically focus an element after removing its focus handler #4867

Closed
JoolsCaesar opened this issue Mar 29, 2021 · 5 comments · Fixed by #4885
Closed

You cannot programmatically focus an element after removing its focus handler #4867

JoolsCaesar opened this issue Mar 29, 2021 · 5 comments · Fixed by #4885

Comments

@JoolsCaesar
Copy link

JoolsCaesar commented Mar 29, 2021

Description

I'm seeing this in the latest chromium browsers Windows Chrome & Edge 89, using jquery 3.6.

If you add and remove an element's focus handler, subsequent calls to $element.focus(), will do nothing.
HTML:

<div tabindex="-1" class="fdiv"></div>

CSS:

.fdiv{
  height:100px;
  width:100px;
  border:1px solid black;
}
.fdiv:focus{
  background-color:green;
}

See the linked to test case below. I have a simple div that turns green when you focus it and some js that will focus said div after 2 seconds.
Before setting this timeout I add and then immediately remove an empty focus handler:

//Using https://cdnjs.cloudflare.com/ajax/libs/jquery/3.6.0/jquery.min.js

const $origDiv = $('.fdiv');

// Comment out the following line to fix
$origDiv.on('focus',function(){}).off('focus');

setTimeout(function(){
	$origDiv.focus();
},2000)

If you run the fiddle, the div should turn green (gain focus) after 2 seconds, but it doesn't!
If you comment out the focus handler line, all works as expected.

Link to test case

https://jsfiddle.net/JoolsCaesar/ut371acd/


EDIT by @mgol: I fixed syntax highlighting in code blocks.

@JoolsCaesar
Copy link
Author

JoolsCaesar commented Mar 29, 2021

FWIW I tracked this down to this bit in jquery:

function leverageNative( el, type, expectSync ) {

	// Missing expectSync indicates a trigger call, which must force setup through jQuery.event.add
	if ( !expectSync ) {
		if ( dataPriv.get( el, type ) === undefined ) {
			jQuery.event.add( el, type, returnTrue );
		}
		return;
	}

dataPriv.get( el, type ), where el is the element we're attempting to focus and type is 'focus', returns false, so we just drop out without doing anything.

@mgol mgol added this to the 3.6.1 milestone Mar 29, 2021
@mgol mgol self-assigned this Mar 29, 2021
@mgol mgol added the Discuss in Meeting Reserved for Issues and PRs that anyone would like to discuss in the weekly meeting. label Mar 29, 2021
@mgol
Copy link
Member

mgol commented Mar 29, 2021

Thanks for the report! The code you mentioned exists since jQuery 3.4.0, I suspect it only became a problem after #4813 landed.

@JoolsCaesar
Copy link
Author

Makes sense. I spotted this after upgrading from 3.0.0 to 3.6.0, so couldn't narrow it down much.

@mgol
Copy link
Member

mgol commented Mar 29, 2021

I meant the bug is only reproducible in jQuery 3.6.0, it works fine in 3.5.1 despite the fact 3.5.1 contains the code you linked to. Hence, most likely it only causes the issue in connection with #4813 (although I haven't verified it yet).

@timmywil timmywil removed the Discuss in Meeting Reserved for Issues and PRs that anyone would like to discuss in the weekly meeting. label Apr 26, 2021
mgol added a commit to mgol/jquery that referenced this issue May 7, 2021
The `_default` function in the special event settings for focus/blur has
always returned `true` since jquerygh-4813 as the event was already being fired
from `leverageNative`. However, that only works if there's an active handler
on that element; this made a quick consecutive call:

```js
elem.on( "focus", function() {} ).off( "focus" );
```

make subsequent `.trigger( "focus" )` calls to not do any triggering.

The solution, already used in a similar `_default` method for the `click` event,
is to check for the `dataPriv` entry on the element for the focus event
(similarly for blur).

Fixes jquerygh-4867
@mgol
Copy link
Member

mgol commented May 7, 2021

PR: #4885

mgol added a commit that referenced this issue May 10, 2021
The `_default` function in the special event settings for focus/blur has
always returned `true` since gh-4813 as the event was already being fired
from `leverageNative`. However, that only works if there's an active handler
on that element; this made a quick consecutive call:

```js
elem.on( "focus", function() {} ).off( "focus" );
```

make subsequent `.trigger( "focus" )` calls to not do any triggering.

The solution, already used in a similar `_default` method for the `click` event,
is to check for the `dataPriv` entry on the element for the focus event
(similarly for blur).

Fixes gh-4867
Closes gh-4885
mgol added a commit to mgol/jquery that referenced this issue May 10, 2021
The `_default` function in the special event settings for focus/blur has
always returned `true` since jquerygh-4813 as the event was already being fired
from `leverageNative`. However, that only works if there's an active handler
on that element; this made a quick consecutive call:

```js
elem.on( "focus", function() {} ).off( "focus" );
```

make subsequent `.trigger( "focus" )` calls to not do any triggering.

The solution, already used in a similar `_default` method for the `click` event,
is to check for the `dataPriv` entry on the element for the focus event
(similarly for blur).

Fixes jquerygh-4867
Closes jquerygh-4885

(cherry picked from commit e539bac)
jwmerrill added a commit to desmosinc/mathquill that referenced this issue Nov 12, 2021
jQuery 3.x has introduced a bunch of different focus bugs in the process
of attempting to migrate to using native focus events in more cases,
some of which remain unfixed in the latest version (3.6.0).

Examples:
* jquery/jquery#4859
* jquery/jquery#4856
* jquery/jquery#4950
* jquery/jquery#4867

Some unknown bug in this general class can make it so that Mathquill
public api focus calls stop working because using jQuery to
programatically trigger focus breaks.

Work around this by switching to triggering native focus events instead
of jQuery focus events.
jwmerrill added a commit to desmosinc/mathquill that referenced this issue Nov 16, 2021
jQuery 3.x has introduced a bunch of different focus bugs in the process
of attempting to migrate to using native focus events in more cases,
some of which remain unfixed in the latest version (3.6.0).

Examples:
* jquery/jquery#4859
* jquery/jquery#4856
* jquery/jquery#4950
* jquery/jquery#4867

Some unknown bug in this general class can make it so that Mathquill
public api focus calls stop working because using jQuery to
programatically trigger focus breaks.

Work around this by switching to triggering native focus events instead
of jQuery focus events.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

3 participants