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

Edge: .is(":focus") and .not(":focus") return wrong values #4109

Closed
komarovalexander opened this Issue Jun 19, 2018 · 15 comments

Comments

Projects
None yet
5 participants
@komarovalexander

komarovalexander commented Jun 19, 2018

Description

code for reproducing:

<!DOCTYPE html>
<html>
<head>
  <title>jQuery test</title>
  <script
    src="https://code.jquery.com/jquery-3.3.1.js"
    integrity="sha256-2Kok7MbOyxpgUVvAk/HJ2jigOSYS2auK4Pfzbm7uH60="
    crossorigin="anonymous"></script>
  <script type="text/javascript">
    $(function(){
	setTimeout(function(){
 	  $("input#first").focus();		
	  alert("input focused: " + $("input").is(":focus")  
	  	+ "\r\n input#first focused: " + $("input#first").is(":focus") 
		+ "\r\n input:focus id: " + $("input:focus").attr("id") 
		+ "\r\n $('input').not(':focus').length: " + $("input").not(":focus").length
		+ "\r\n $('input:not(:focus)').length: " + $("input:not(:focus)").length
		+ "\r\n focused count: " + $("input:focus").length);
	}, 1000);
  });
  </script>
</head>
<body>
  <input type="text" id="first" name="a">
  <input type="text" id="second" name="b">
</body>
</html>

When I use Edge I have strange behavior of .is(":focus") and .not(".focus")

How it should work?

all browsers:
image

edge (if you click by page after reload):
image

What is wrong?
edge: if you do not click by page after reload, you will get these results (you can click by page tab if it is not reproduced):

image

element is focused and selectors fetch it. But if you use .is() and .not() you will get wrong results

I reproduced it in my tests and I have to spend time to find reasons why sometimes it is working and sometimes is not working

Which browsers are affected?

Microsoft Edge

my version is
Microsoft Edge 42.17134.1.0
Microsoft EdgeHTML 17.17134

other browsers are ok.

test case is attached
test_to_reproduce.zip

@komarovalexander komarovalexander changed the title from Edge: .is(":focus") and .not(".focus") return wrong values to Edge: .is(":focus") and .not(":focus") return wrong values Jun 19, 2018

@timmywil

This comment has been minimized.

Member

timmywil commented Jul 2, 2018

Thanks for opening an issue. However, I copied the test case to jsfiddle and I cannot reproduce. https://jsfiddle.net/63zby1nt/show

@timmywil

This comment has been minimized.

Member

timmywil commented Jul 2, 2018

Without reproducing, there's not much we can do. :focus can be finicky sometimes. Even with a reproducible test case, there may not be anything we can do to fix it.

@timmywil timmywil closed this Jul 2, 2018

@fastfasterfastest

This comment has been minimized.

fastfasterfastest commented Jul 2, 2018

Thanks for opening an issue. However, I copied the test case to jsfiddle and I cannot reproduce. https://jsfiddle.net/63zby1nt/show

FWIW, that fiddle reproduces the reported behavior in Edge 42.17134.1.0 for me - when page is refreshed it logs

input focused: false
input#first focused: false
input:focus id: first
$('input').not(':focus').length: 2
$('input:not(:focus)').length: 1
focused count: 1
@dmethvin

This comment has been minimized.

Member

dmethvin commented Jul 2, 2018

@fastfasterfastest I see that result as well. However, it's possible that Edge still has the async-focus behavior that the old IEs had. Focus didn't occur until the next event loop turn. If I change it to $("input#first")[0].focus(); I get the result I think you expect.

@fastfasterfastest

This comment has been minimized.

fastfasterfastest commented Jul 2, 2018

I hadn't really studied the issue reported and was just commenting on @timmywil statement he couldn't reproduce the reported behavior, while I could, even with @timmywil's fiddle.

If I change it to $("input#first")[0].focus(); I get the result I think you expect.

What is the result you get and what is the result you expect?
If I change to $("input#first")[0].focus() there is no difference in the logging.

If I change to do the logging in a setTimeout in an attempt to avoid an async-focus issue that may come into play (but I don't really know much about the async-focus behavior issue old IE had and may be misunderstanding what you are referring to), I see no difference in the logging - it still logs the same, see https://jsfiddle.net/63zby1nt/4/show

(It appears when you refresh the page, Edge does not actually cause the focus to be set to the input element at all...)

@dmethvin

This comment has been minimized.

Member

dmethvin commented Jul 2, 2018

When I first used the original fiddle I got your results.

When I used the updated fiddle I got:

input focused: true
 input#first focused: true
 input:focus id: first
 $('input').not(':focus').length: 1
 $('input:not(:focus)').length: 1
 focused count: 1

Now I get the true etc results, even on the original fiddle. Something is fishy with Edge, but we're doing the best we can with the information we have. You might try creating an example not using jQuery and report it to Edge.

@fastfasterfastest

This comment has been minimized.

fastfasterfastest commented Jul 3, 2018

Now I get the true etc results, even on the original fiddle.

I did too - until I realized I was using Chrome... maybe you did too? Using Edge, I get consistent results. And you must refresh the page per @komarovalexander's report to see the reported behavior.

Using the updated fiddle in Edge, I initially get (prior to the refresh):

input focused: true
 input#first focused: true
 input:focus id: first
 $('input').not(':focus').length: 1
 $('input:not(:focus)').length: 1
 focused count: 1

When the page is refreshed I get:

input focused: false
 input#first focused: false
 input:focus id: first
 $('input').not(':focus').length: 2
 $('input:not(:focus)').length: 1
 focused count: 1
@fastfasterfastest

This comment has been minimized.

fastfasterfastest commented Jul 3, 2018

You might try creating an example not using jQuery and report it to Edge.

Note, I was not the original reporter.

Nevertheless, here is a fiddle without jquery and it seems to behave "correctly" - https://jsfiddle.net/63zby1nt/9/show
(Added: except that after the refresh the input element doesn't actually have the focus - the caret is not in the input element. But DOM calls seem to indicate the element has focus. Perhaps this is what you meant by reporting it to Edge?)

Here is a fiddle w/ jquery and some querySelectorAll calls to attempt to "simulate" the jquery calls - https://jsfiddle.net/63zby1nt/18/show Look for the "input focused" and "not focus count" lines - they show differences in the jquery vs. non-jquery values. I don't know if I got the "translation" to non-jquery correct, though.

input focused - $('input').is(':focus'): false
input focused - qsa('input')~.any(':focus'):true
input#first focused: false
input:focus id: first
not focus count - $('input').not(':focus').length: 2
not focus count - qsa('input')~.length:1
qsa('input:not(:focus)').length: 1
$('input:not(:focus)').length: 1
focused count - $('input:focus').length: 1
focused count - qsa('input:focus').length: 1

(Also strange (?) is that the focus is not actually in the input element after the refresh - the caret is not in the input element.)

@dmethvin

This comment has been minimized.

Member

dmethvin commented Jul 3, 2018

Even the /show URL puts the code inside an iframe. We've seen iframe-related focus problems with IE in the past, maybe Edge inherited those.

That said, I opened Edge, pasted in the URL, and hit enter. This is the result I got:

input focused - $('input').is(':focus'): true
input focused - qsa('input')~.any(':focus'):true
input#first focused: true
input:focus id: first
not focus count - $('input').not(':focus').length: 1
not focus count - qsa('input')~.length:1
qsa('input:not(:focus)').length: 1
$('input:not(:focus)').length: 1
focused count - $('input:focus').length: 1
focused count - qsa('input:focus').length: 1

capture

@fastfasterfastest

This comment has been minimized.

fastfasterfastest commented Jul 3, 2018

That said, I opened Edge, pasted in the URL, and hit enter. This is the result I got

Again, you need to refresh the page and/to see the issue. Do what you did, then refresh the page.

@fastfasterfastest

This comment has been minimized.

fastfasterfastest commented Jul 4, 2018

It seems like the issue the OP is reporting may be due to that Edge does not "fully" set focus to the input element after the page has been reloaded using the browser's "Refresh" button. When the page is reloaded using the browser's "Refresh" button it appears that the document is not given focus, as seen by probing document.hasFocus(), resulting in some weirdness e.g. the input caret is not in the input element even though the input element reports it has focus.

Nevertheless, I was curious and looked into this a little bit and noticed something interesting/weird, perhaps a bug, in jquery in this particular situation. It is not necessarily causing the issue the OP reports, but may contribute to some confusion.

I noticed that the value of $('input:focus').is(':focus') depends on other calls to is.

In the sample below, $('input:focus').is(':focus') (initially) returns true.
Then a call to $('input').is(':focus') is made and returns false (*see below).
From then on, the same call to $('input:focus').is(':focus') now returns false.

This is due to that two different code paths are taken in jquery depending on whether $(selector) contains one or more elements. If $(selector) contains one element then one code path is taken to determine the value of is using the DOM matches method. If $(selector) contains more than one element, a different code path is taken, into Sizzle, and a different way of determining the value of is is used. Unfortunately, those two code paths return different values (for the same element) under some circumstances (in particular for .is(':focus') when document.hasFocus() returns false). Furthermore, once the different code path into Sizzle has been taken for an element, then future calls will also take that path, even if $(selector) contains one element only.

The result is $(selector).is(':focus') can return one value at one time, and another value later without any changes in focus taking place - just because two different methods of calculating the value exist and were used.

(*: This is strange by itself from a logic point of view - $('input').is(':focus') returns false when there exists an input element that does have focus as evidenced by $('input:focus').is(':focus') returning true.)

Here is the sample that demonstrates this - I provide it, not as a fiddle to avoid any iframe issues coming into play, as code you have to save to a file and load in Edge.

<!DOCTYPE html>
<html>
<head>
    <title>jQuery test</title>
    <script src="https://code.jquery.com/jquery-3.3.1.js"
            integrity="sha256-2Kok7MbOyxpgUVvAk/HJ2jigOSYS2auK4Pfzbm7uH60="
            crossorigin="anonymous"></script>
    <script type="text/javascript">
        $(function () {
            $("input#first").focus();

            setTimeout(function () {
                var inputPseudoFocusIsPseudoFocus = $('input:focus').is(':focus');

                document.getElementById("log").textContent += "" +
                    "$('input:focus').is(':focus')=" + $('input:focus').is(':focus') + "\r\n" +
                    "$('input').is(':focus')=" + $('input').is(':focus') + "\r\n" +
                    "$('input:focus').is(':focus')=" + $('input:focus').is(':focus') +
                    (inputPseudoFocusIsPseudoFocus !== $('input:focus').is(':focus') ? "  - NOTE: value changed after $('input').is(':focus') was called!!" : "") + "\r\n" +
                    "";
            }, 100);

            if (window.navigator.userAgent.indexOf('Edge/') === -1) {
                document.querySelector("body").insertAdjacentHTML('afterbegin', '<h1 style="color:red">This should be done using Edge</h1>');
            }
        });
    </script>
</head>
<body>
    <div>
        Reload page using browser's "Refresh" button and notice how value of $('input:focus').is(':focus') is affected by a call to $('input').is(':focus')<br/><br />
        Reload page using F5 or clicking <button onclick="window.location.reload(true)">Reload</button> and notice how value of $('input:focus').is(':focus') is NOT affected by a call to $('input').is(':focus')<br /><br />
    </div>

    <form>
        <input type="text" id="first" name="a">
        <input type="text" id="second" name="b">
    </form>
    <pre id="log"></pre>
</body>
</html>
@fastfasterfastest

This comment has been minimized.

fastfasterfastest commented Jul 4, 2018

I think there is an issue with jquery/Sizzle here, regardless of whether Edge "fully" sets focus after a reload or not. jquery uses two different ways to determine the value of .is(':focus') - one way uses Edge's notion of :focus and the other way uses Sizzle's notion of what :focus means. And Sizzle's notion does not correspond to Edge's notion, which I think is an issue.

So to restate, hopefully little clearer...

For sample code demonstrating the issue, see preceding comment above #4109 (comment)

Condition: using Edge and page must be reloaded by using browser's "Refresh" button. Note, the page must be reloaded using browser's "Refresh" button to demonstrate the issue.

Assume the document has an input element inputWithFocus such that inputWithFocus.matches(':focus') returns true. That means, one would expect $(inputWithFocus).is(':focus') to return true.

First issue is that the value of $('input').is(':focus') depends on how many elements $('input') matches:

  • if $('input') matches one element, then $('input').is(':focus') returns true, as expected
  • if $('input') matches more than one element, then $('input').is(':focus') returns false, unexpectedly

$('input').is(':focus') returning false is unexpected because there is an input element for which $(element).is(':focus') would have returned true

Second issue is that if $('input') matches more than one element, then merely calling $('input').is(':focus') affects the value $(inputWithFocus).is(':focus') will return:

  1. initially, call $(inputWithFocus).is(':focus') and it will return true, as expected
  2. then, call $('input').is(':focus') and it will return false, unexpectedly
  3. then, call $(inputWithFocus).is(':focus') again and it will now return false, doubly unexpectedly

$(inputWithFocus).is(':focus') returning false is doubly unexpectedly because:

  • it returned true prior to calling $('input').is(':focus'), and now returns false
  • inputWithFocus.matches(':focus') returns true

(I don't know if this should be opened as an issue at https://github.com/jquery/sizzle/issues instead)

@dmethvin

This comment has been minimized.

Member

dmethvin commented Jul 8, 2018

Thanks for the analysis! I think it''s fine to file it here for now until we figure out whether and how this can be fixed.

@dmethvin dmethvin reopened this Jul 8, 2018

@810

This comment has been minimized.

810 commented Jul 9, 2018

I think the issue has been resolved:

test

@timmywil

This comment has been minimized.

Member

timmywil commented Jul 10, 2018

Thank you for the thorough analysis. I'm almost positive, tho, that this issue should be migrated to Sizzle, so here we go: jquery/sizzle#424

@timmywil timmywil closed this Jul 10, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment