Jquery: bug with focus() method in a specific case in 3.4.0

Created on 1 May 2019 · 9Comments · Source: jquery/jquery

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

Blocker Bug Event Has Pull Request

Source

zdravkov

👀1

Most helpful comment

@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.

mgol on 1 May 2019

❤1 👍1

All 9 comments

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

mgol on 1 May 2019

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

timmywil on 1 May 2019

mgol on 1 May 2019

❤1 👍1

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.

caseyjhol on 15 May 2020

https://github.com/jquery/jquery/blob/6984d1747623dbc5e87fd6c261a5b6b1628c107c/src/event.js#L185-L191

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 on 23 Nov 2020

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
https://github.com/jquery/jquery/blob/e1cffdef277fcf543833a20d28cbadcd000ebece/src/event/focusin.js#L37

So elem.addEventListener(type, eventHandle) in the event.js file does not need to listen to the focus event.
https://github.com/jquery/jquery/blob/e1cffdef277fcf543833a20d28cbadcd000ebece/src/event.js#L205-L211

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 on 1 Dec 2020

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>

ygj6 on 1 Dec 2020

@ygj6 The original test case passes on master but the one from https://github.com/jquery/jquery/issues/4382#issuecomment-629269540 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:
https://github.com/jquery/jquery/blob/6984d1747623dbc5e87fd6c261a5b6b1628c107c/src/event.js#L738
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.

mgol on 1 Dec 2020

PR: #4813

mgol on 1 Dec 2020

🎉1

Was this page helpful?

0 / 5 - 0 ratings