bug with focus() method in a specific case in 3.4.0 #4382

zdravkov · 2019-05-01T10:26:26Z

Hi all,

I just noticed and odd change in the behavior of the focus method in jQuery 3.4.0 compared with 3.3.1, 2.**, and 1.. versions in a specific case when we try to change the focus to the child element in focusin event of its parent. Here is the code and here you can see a code dojo with the issue:

        <span>Check the console</span>
        <div id="toolbar" tabindex="0">
                 <a tabIndex="0" id="foo" >foo</a>
        </div>
        <script>
            $(document).ready(function () {
                var toolbar = $('#toolbar');

                toolbar.on("focusin", function (ev) {
                        var element = $(this).find("a");

                        if (element.length) {
                            element[0].focus();
                            console.log(document.activeElement)
                        }
                    })

                toolbar[0].focus();
                //toolbar[0].focus(); this works

                console.log('With 3.3.1 here we expect to see "foo":', document.activeElement.id);

            });
        </script>

If we use older version or the native focus() method the issue is not present.

Greetings.
Plamen

The text was updated successfully, but these errors were encountered:

mgol · 2019-05-01T12:44:26Z

Thanks for the report. The issue doesn't exist on master (future jQuery 4.0) where we removed our custom focusin patch but it still exists on 3.4-stable.

cc @gibson042

timmywil · 2019-05-01T15:36:05Z

@jquery/core Is this something we want to get into 3.4.1?

mgol · 2019-05-01T17:26:56Z

@timmywil While I think this issue should be fixed in a patch update to 3.4, IMO #4350 & #4356 are way more serious and it's been 3 weeks since the 3.4.0 release so I'd rather not postpone 3.4.1 further. We can always do 3.4.2 with a fix for this issue & any other that may pop up in the meantime.

caseyjhol · 2020-05-15T14:34:28Z

More examples here:

jQuery v3.3.1 (working): https://fresh-capable-sense.glitch.me/jquery-3.3.1.html
jQuery v3.4.0 (not working): https://fresh-capable-sense.glitch.me/jquery-3.4.0.html
jQuery v3.5.1 (not working): https://fresh-capable-sense.glitch.me/jquery-3.5.1.html
jQuery master (not working): https://fresh-capable-sense.glitch.me/jquery-master.html
Editor: https://glitch.com/edit/#!/fresh-capable-sense

The "Focus this" button element should be focused.

ygj6 · 2020-11-23T09:33:07Z

jquery/src/event.js

Lines 185 to 191 in 6984d17

    
           if ( !special.setup || 
        
           	special.setup.call( elem, data, namespaces, eventHandle ) === false ) { 
        
           	if ( elem.addEventListener ) { 
        
           		elem.addEventListener( type, eventHandle ); 
        
           	} 
        
           }

I researched this issue and found that when jQuery executes the toolbar.focus(), jQuery binds a native focus event (elem.addEventListener( type, eventHandle )) to the element toolbar, After toolbar gets the focus, it first triggers the event bound to toolbar.on("focusin",...), so that the element foo gets the focus, and then it triggers elem.addEventListener( type, eventHandle ) makes the focus from the element foo back to the toolbar.

I tried to delete elem.addEventListener( type, eventHandle ); and the problem was solved.

ygj6 · 2020-12-01T08:21:35Z

I found that the latest master branch did not have this problem, I analyzed the issue based on jquery3.5.1.
In the focusin.js file, listened to the focus event

jquery/src/event/focusin.js

Line 37 in e1cffde

doc.addEventListener( orig, handler, true );

So elem.addEventListener(type, eventHandle) in the event.js file does not need to listen to the focus event.

jquery/src/event.js

Lines 205 to 211 in e1cffde

    
           if ( !special.setup || 
        
           	special.setup.call( elem, data, namespaces, eventHandle ) === false ) { 
        
           	if ( elem.addEventListener ) { 
        
           		elem.addEventListener( type, eventHandle ); 
        
           	} 
        
           }

I modified the source code and added conditions,

if (elem.addEventListener && type != "focus" ){
	elem.addEventListener(type,eventHandle);
}

After the modification, the problem is fixed. Is this correct?

@mgol

ygj6 · 2020-12-01T09:36:29Z

Add testcase

<!DOCTYPE html>
<html>
<head>
    <!--<script src="http://code.jquery.com/jquery-git.js"></script>-->
	<script src="http://code.jquery.com/jquery-3.5.1.js"></script>
</head>
<body onload="aa()">
         <span>Check the console</span>
		 <div id="toolbar" tabindex="0" style="width: 200px;display: inline-block;">
            <a tabIndex="0" id="foo" >foo</a>     
        </div>
        
        <script type="text/javascript">
            aa = function(){ 
                var toolbar = $('#toolbar');
                toolbar.on("focusin", function (ev) {
					var element = $("#foo");

					if (element.length) {
						element[0].focus();
					}
				});
                
                toolbar.focus();
            }
			
			setTimeout(function(){
				console.log(document.activeElement.id);
			},1000);
        
        </script>
</body>
</html>

mgol · 2020-12-01T09:42:23Z

@ygj6 The original test case passes on master but the one from #4382 (comment) by @caseyjhol doesn't, the issue is there as well.

Your proposed fix won't work, unfortunately. You can run the test suite, I expect multiple failures here. This addEventListener is important, if it wasn't needed we'd be able to just change this line to true:

jquery/src/event.js

Line 738 in 6984d17

return false;

The issue is only with multiple focus handlers & a re-trigger in the middle and we need to handle that. I have a fix in progress, I'll try to submit a PR today.

If during a focus handler another focus event is triggered: ```js elem1.on( "focus", function() { elem2.trigger( "focus" ); } ); ``` due to their synchronous nature everywhere outside of IE the hack added in jquerygh-4279 to leverage native events causes the native `.focus()` method to be called last for the initial element, making it steal the focus back. To resolve this, we now skip calling the native method if focus was changed. A side effect of this change is that now `focusin` will only propagate to the document for the last focused element. This is a change in behavior but it also aligns us better with how this works with native methods. Fixes jquerygh-4382 Ref jquerygh-4279

mgol · 2020-12-01T12:20:42Z

PR: #4813

If during a focus handler another focus event is triggered: ```js elem1.on( "focus", function() { elem2.trigger( "focus" ); } ); ``` due to their synchronous nature everywhere outside of IE the hack added in jquerygh-4279 to leverage native events causes the native `.focus()` method to be called last for the initial element, making it steal the focus back. To resolve this, we now skip calling the native method if focus was changed. A side effect of this change is that now `focusin` will only propagate to the document for the last focused element. This is a change in behavior but it also aligns us better with how this works with native methods. Fixes jquerygh-4382 Ref jquerygh-4279

If during a focus handler another focus event is triggered: ```js elem1.on( "focus", function() { elem2.trigger( "focus" ); } ); ``` due to their synchronous nature everywhere outside of IE the hack added in gh-4279 to leverage native events causes the native `.focus()` method to be called last for the initial element, making it steal the focus back. Since the native method is already being called in `leverageNative`, we can skip that final call. This aligns with changes to the `_default` method for the `click` event that were added when `leverageNative` was introduced there. A side effect of this change is that now `focusin` will only propagate to the document for the last focused element. This is a change in behavior but it also aligns us better with how this works with native methods. Fixes gh-4382 Closes gh-4813 Ref gh-4279

If during a focus handler another focus event is triggered: ```js elem1.on( "focus", function() { elem2.trigger( "focus" ); } ); ``` due to their synchronous nature everywhere outside of IE the hack added in gh-4279 to leverage native events causes the native `.focus()` method to be called last for the initial element, making it steal the focus back. Since the native method is already being called in `leverageNative`, we can skip that final call. This aligns with changes to the `_default` method for the `click` event that were added when `leverageNative` was introduced there. A side effect of this change is that now `focusin` will only propagate to the document for the last focused element. This is a change in behavior but it also aligns us better with how this works with native methods. Fixes gh-4382 Closes gh-4813 Ref gh-4279 (cherry picked from commit dbcffb3)

Focus re-triggering in jQuery 3.4/3.5 makes the original element have its focus event propagated last, breaking the re-targeting. Trigger focus in a delay in addition if needed to avoid the issue. This fixes the "interaction between overlay and other dialogs" core dialog test when tested against jQuery 3.4/3.5. Ref jquery/jquery#4382

Focus re-triggering in jQuery 3.4/3.5 makes the original element have its focus event propagated last, breaking the re-targeting. Trigger focus in a delay in addition if needed to avoid the issue. This fixes the "interaction between overlay and other dialogs" core dialog test when tested against jQuery 3.4/3.5. Closes gh-1946 Ref jquery/jquery#4382

Workaround see select2/select2#5993 and jquery/jquery#4382

* fix spelling * autofocus searchfield in Select2 Workaround see select2/select2#5993 and jquery/jquery#4382 * fix sizing of the Information seat-type

* fix spelling * autofocus searchfield in Select2 Workaround see select2/select2#5993 and jquery/jquery#4382

zdravkov changed the title ~~bug with focus() method in a specific case~~ bug with focus() method in a specific case in 3.4.0 May 1, 2019

mgol added Bug Event Needs review labels May 1, 2019

mgol added this to the 3.4.2 milestone May 4, 2019

timmywil removed the Needs review label May 13, 2019

timmywil added Needs review and removed Needs review labels Dec 2, 2019

timmywil added the Blocker label Jan 20, 2020

mgol modified the milestones: 3.5.0, 3.6.0 Mar 16, 2020

caseyjhol mentioned this issue May 15, 2020

Programmatically set focus on select snapappointments/bootstrap-select#1212

Closed

mgol self-assigned this Dec 1, 2020

mgol mentioned this issue Dec 1, 2020

Event: Make focus re-triggering not focus the original element back #4813

Merged

3 tasks

mgol added the Has Pull Request label Dec 1, 2020

mgol closed this as completed in #4813 Dec 7, 2020

mgol mentioned this issue Feb 21, 2021

Dialog: Workaround broken focus re-triggering in jQuery 3.4/3.5 jquery/jquery-ui#1946

Merged

XhmikosR mentioned this issue Mar 2, 2021

Update jQuery to v3.6.0. twbs/bootstrap#33243

Closed

tagliala mentioned this issue Mar 14, 2021

Search not auto focusing in jQuery 3.6.0 select2/select2#5993

Closed

aarongranick-okta mentioned this issue Jul 1, 2021

Upgrade jQuery to latest version 3.6.0 okta/okta-signin-widget#2075

Closed

4 tasks

firegore added a commit to KRRUg/KLMS that referenced this issue Oct 13, 2021

autofocus searchfield in Select2

63c47fb

Workaround see select2/select2#5993 and jquery/jquery#4382

firegore added a commit to KRRUg/KLMS that referenced this issue Oct 13, 2021

Fix/seatmap information sizing (#72)

df1c805

* fix spelling * autofocus searchfield in Select2 Workaround see select2/select2#5993 and jquery/jquery#4382 * fix sizing of the Information seat-type

firegore added a commit to KRRUg/KLMS that referenced this issue May 16, 2022

Fix/qol fixes (#71)

57f0d40

* fix spelling * autofocus searchfield in Select2 Workaround see select2/select2#5993 and jquery/jquery#4382

gguerini mentioned this issue May 31, 2022

Upgrade jQuery from 3.5.1 to 3.6.0. railsadminteam/rails_admin#3524

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

bug with focus() method in a specific case in 3.4.0 #4382

bug with focus() method in a specific case in 3.4.0 #4382

zdravkov commented May 1, 2019

mgol commented May 1, 2019

timmywil commented May 1, 2019

mgol commented May 1, 2019

caseyjhol commented May 15, 2020 •

edited

Loading

ygj6 commented Nov 23, 2020

ygj6 commented Dec 1, 2020

ygj6 commented Dec 1, 2020

mgol commented Dec 1, 2020

mgol commented Dec 1, 2020

bug with focus() method in a specific case in 3.4.0 #4382

bug with focus() method in a specific case in 3.4.0 #4382

Comments

zdravkov commented May 1, 2019

mgol commented May 1, 2019

timmywil commented May 1, 2019

mgol commented May 1, 2019

caseyjhol commented May 15, 2020 • edited Loading

ygj6 commented Nov 23, 2020

ygj6 commented Dec 1, 2020

ygj6 commented Dec 1, 2020

mgol commented Dec 1, 2020

mgol commented Dec 1, 2020

caseyjhol commented May 15, 2020 •

edited

Loading