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

:focus selector inconsistent in Edge Legacy #424

Open
timmywil opened this issue Jul 10, 2018 · 10 comments
Open

:focus selector inconsistent in Edge Legacy #424

timmywil opened this issue Jul 10, 2018 · 10 comments
Labels
Has a jQuery counterpart Issues that have a related jQuery issue; used to identify issues relevant after Sizzle is archived

Comments

@timmywil
Copy link
Member

Migrated from jquery/jquery#4109

Test case: https://jsfiddle.net/63zby1nt/

Seems like maybe the process the of checking for focus in Edge causes the input to lose focus. More thorough analysis is available in the comments on the jQuery issue.

@fastfasterfastest
Copy link

fastfasterfastest commented Jul 10, 2018

maybe the process the of checking for focus in Edge causes the input to lose focus

I don't think so - I don't think the input looses the focus at all the issue is about the input loosing focus.

As I state in jquery/jquery#4109 (comment) (and the sample in jquery/jquery#4109 (comment) shows) it appears that jquery has two different ways of determining the value of .is(':focus) - one way ("native path") is used if the jquery object contains one element and uses the native .matches(':focus'); another way ("Sizzle path") is used when the jquery object contains two or more elements and uses Sizzle's notion of what :focus means. However, Sizzle's notion of what :focus means does not correspond to the native notion in this particular case.

In addition, when the "Sizzle path" is taken for an element, it appears that Sizzle caches that decision for the element and the "native path" will no longer be taken for that element.

Thus:
1- $(input#foo).is(':focus') can return true while $('input').is(':focus') returns false
2- $(input#foo).is(':focus') can return true at one moment and false at a later moment (without focusing changing) if $('input').is(':focus') is called in-between

@timmywil
Copy link
Member Author

I should have been more clear, by "lose focus", I meant the activeElement property gets unset or the page loses focus. Has nothing to do with whether the cursor is actually still in the input.

@fastfasterfastest
Copy link

I don't think it is anything "dynamic" - I don't think document.activeElement gets unset.

I think it is simply that Edge's notion of .matches(':focus') is different than Sizzle's notion in this particular case: Edge's notion returns true even if document.hasFocus() returns false, Sizzle's notion returns... well it depends....

Actually, Sizzle's notion depends on if the jquery object contains one element or if it contains more than one element. If it contains one element then Sizzle's notion also returns true (because it defers to the native .matches(':focus'))[*1], but if it contains more than one element then it returns false because Sizzle also takes document.hasFocus() into account.

[*1]: with the caveat in item 2 above - Sizzle's notion may return true or false depending on whether .is(':focus') has been called with a jquery object containing more than one element or not...

@timmywil
Copy link
Member Author

timmywil commented Jul 10, 2018

Again, it was just a theory. We've seen situations where simply accessing a property in IE or Edge has unexpected side effects. It seemed conceivable that something like that was happening here when hitting document.activeElement. That said, it sounds like you've figured out this has more to do with hasFocus.

Whether it's checking more than one element is not really the issue. It may be a factor in whether the native .matches gets hit, but that is simply exposing the underlying problem when the path does go through Sizzle. Since .matches seems to work fine, I'm only concerned with the Sizzle path.

In other words, when to use native matches and when to use Sizzle is another conversation.

@fastfasterfastest
Copy link

Whether it's checking more than one element is not really the issue.

And I have not stated that is the issue. The issue, I think, is that there currently are two different ways/paths to determine the value of .is(':focus') and they return different values. I show how one can get these two different values - namely whether the jquery object contains one element or more than one element.

Since .matches seems to work fine, I'm only concerned with the Sizzle path.

I am not sure .matches work fine - does it? Is it correct that .matches(':focus') returns true in Edge when document.hasFocus() returns false?
(It seems like the root of the issue is that Edge does not set focus to the document after the user clicks the "Reload" button and the document is reloaded. Edge does so when you type in the url in the address bar, but not when you click the "Reload", "Back" and "Forward" buttons. If it did set the focus to the document this whole issue would be moot because then the Sizzle path would correspond with the native path.)
Regardless, it seems like it would be a good thing if the two different ways/paths returned the same value. And perhaps that value should be, not what .matches returns, but instead what Sizzle returns?

Here is another sample that shows the current behavior.

var matchesFocus1 = document.querySelector('input#first').matches(':focus'),
    isFocus1 = $('input#first').is(':focus'),
    isFocus2 = $('input').is(':focus'),
    isFocus3 = $('input#first').is(':focus');

Assume matchesFocus1===true. Then I would say the following is expected:

  • isFocus1===true because document.querySelector('input#first').matches(':focus')===true
  • isFocus2===true because the element document.querySelector('input#first') is among the elements matched by $('input')
  • isFocus3===true because document.querySelector('input#first').matches(':focus')===true AND ALSO because $('input#first').is(':focus') returned true two lines above and no intervening focus change occurred.

But currently, in this particular scenario (when the page is reloaded by clicking browsers "Reload" button, the following is observed:

  • isFocus1===true
  • isFocus2===false
  • isFocus3===false.
<!DOCTYPE html>
<html>
<head>
    <title>Sizzle issue 424 test</title>
    <script src="https://code.jquery.com/jquery-3.3.1.js"
            integrity="sha256-2Kok7MbOyxpgUVvAk/HJ2jigOSYS2auK4Pfzbm7uH60="
            crossorigin="anonymous"></script>
    <script type="text/javascript">
        window.addEventListener("load", function (event) {
            if (window.navigator.userAgent.indexOf('Edge/') === -1) {
                document.querySelector("body").insertAdjacentHTML('afterbegin', '<h1 style="color:red">This should be done using Edge</h1>');
            }

            window.addEventListener("focusin", function (event) {
                document.getElementById("log").textContent += "event - " + event.type + " - " + event.target.tagName + "#" + event.target.id + "\n" +
                    "   document.hasFocus()=" + document.hasFocus() + "\n" +
                    "   document.activeElement.id=" + (document.activeElement ? document.activeElement.id : "") + "\n";
            });
            window.addEventListener("focusout", function (event) {
                document.getElementById("log").textContent += "event - " + event.type + " - " + event.target.tagName + "#" + event.target.id + "\n" +
                    "   document.hasFocus()=" + document.hasFocus() + "\n" +
                    "   document.activeElement.id=" + (document.activeElement ? document.activeElement.id : "") + "\n";
            });

            document.getElementById("log").textContent += "event - load" + "\n" +
                "   document.hasFocus()=" + document.hasFocus() + "\n" +
                "   document.activeElement.id=" + (document.activeElement ? document.activeElement.id : "") + "\n";

            setTimeout(function () {
                document.getElementById("log").textContent += "Prior to setting focus to input#first\n" +
                    "   document.hasFocus()=" + document.hasFocus() + "\n" +
                    "   document.activeElement.id=" + (document.activeElement ? document.activeElement.id : "") + "\n";

                document.getElementById("log").textContent += "Setting focus to input#first\n";
                document.querySelector("input#first").focus();

                setTimeout(function () {
                    var documentHasFocus1 = document.hasFocus(),
                        activeElementId1 = document.activeElement ? document.activeElement.id : "",
                        matchesFocus1 = document.querySelector('input#first').matches(':focus'),
                        isFocus1 = $('input#first').is(':focus'),
                        documentHasFocus2 = document.hasFocus(),
                        activeElementId2 = document.activeElement ? document.activeElement.id : "",
                        matchesFocus2 = document.querySelector('input#first').matches(':focus'),
                        isFocus2 = $('input').is(':focus'),
                        documentHasFocus3 = document.hasFocus(),
                        activeElementId3 = document.activeElement ? document.activeElement.id : "",
                        matchesFocus3 = document.querySelector('input#first').matches(':focus'),
                        isFocus3 = $('input#first').is(':focus');

                    document.getElementById("log").textContent += "After setting focus to input#first\n" +
                        "   document.hasFocus()=" + documentHasFocus1 + "\n" +
                        "   document.activeElement.id=" + activeElementId1 + "\n" +
                        "   document.querySelector('input#first').matches(':focus')=" + matchesFocus1 + "   *** matchesFocus1 ***" + "\n" +
                        "   $('input#first').is(':focus')=" + isFocus1 + "                             *** isFocus1 ***" + (isFocus1 !== matchesFocus1 ? " - UNEXPECTED - expected " + matchesFocus1 : "") + "\n" +
                        "\n" +
                        "   document.hasFocus()=" + documentHasFocus2 + "\n" +
                        "   document.activeElement.id=" + activeElementId2 + "\n" +
                        "   document.querySelector('input#first').matches(':focus')=" + matchesFocus2 + "\n" +
                        "   $('input').is(':focus')=" + isFocus2 + "                                  *** isFocus2 ***" + (isFocus2 !== isFocus1 ? " - UNEXPECTED - expected " + isFocus1 : "") + "\n" +
                        "\n" +
                        "   document.hasFocus()=" + documentHasFocus3 + "\n" +
                        "   document.activeElement.id=" + activeElementId3 + "\n" +
                        "   document.querySelector('input#first').matches(':focus')=" + matchesFocus3 + "\n" +
                        "   $('input#first').is(':focus')=" + isFocus3 + "                            *** isFocus3 ***" + (isFocus3 !== matchesFocus3 ? " - UNEXPECTED - expected " + matchesFocus3 : "") + "\n" +
                        "";

                    document.querySelector("button").disabled = false;
                }, 100);
            }, 100);
        });
    </script>
</head>
<body>
    <button onclick="window.location.reload(true)" disabled="disabled">Reload</button><br /><br />
    <form>
        <input type="text" id="first" name="a">
        <input type="text" id="second" name="b">
    </form>
    <pre id="log"></pre>
</body>
</html>

@timmywil
Copy link
Member Author

I'm sorry, but you're still not understanding what I'm saying.

The issue, I think, is that there currently are two different ways/paths to determine the value of .is(':focus') and they return different values.

This is the same thing I said just in different words. There are two paths. One is native and one is Sizzle. We can only control the Sizzle path. Once we get that right, we can make sure native is working right. If it's not, as you suspect, there are ways we can ensure that the Sizzle path is always used in edge.

Anyway, I don't think we need to keep having a conversation here. We will investigate and fix the inconsistency.

@fastfasterfastest
Copy link

No need to be sorry - I understand it all right. And am glad you do too now.

@melloware
Copy link

The old Edge is retired and the new Edge is chromium based so not even sure its worth investigating this issue @timmywil ??

@mgol
Copy link
Member

mgol commented Dec 9, 2020

@melloware Sizzle supports and will support Edge Legacy. That said, Sizzle is getting close to being archived; jQuery 4.0 will embed a selector engine ported from Sizzle with workarounds for legacy browsers stripped out; we won't care about Edge Legacy there.

This issue may remain open; if someone submits a fix, we'll review it. It's unlikely anyone from the jQuery team will look into fixing it by themselves, though.

@mgol mgol changed the title :focus selector inconsistent in Edge :focus selector inconsistent in Edge Legacy Sep 7, 2023
@mgol
Copy link
Member

mgol commented Sep 7, 2023

jQuery version of this issue: jquery/jquery#4109

@mgol mgol added the Has a jQuery counterpart Issues that have a related jQuery issue; used to identify issues relevant after Sizzle is archived label Sep 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Has a jQuery counterpart Issues that have a related jQuery issue; used to identify issues relevant after Sizzle is archived
Development

No branches or pull requests

4 participants