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

Stop shimming focusin/focusout in jQuery 4.0 #4300

Closed
mgol opened this issue Feb 18, 2019 · 6 comments · Fixed by #4362
Closed

Stop shimming focusin/focusout in jQuery 4.0 #4300

mgol opened this issue Feb 18, 2019 · 6 comments · Fixed by #4362

Comments

@mgol
Copy link
Member

mgol commented Feb 18, 2019

Description

Firefox has added long-missing support for focusin/focusout events in version 63. Other browsers we plan to support in v4 already support the events (apart from Firefox ESR 60 but we'll drop it before jQuery 4.0 is released).

Currently, we shim focusin/focusout in all non-IE browsers both to add support in Firefox and because order of these events is not implemented according to the spec in various browsers. It used to be wild out there but for now the situation is as follows:

Edge 17+, Chrome, Firefox & Safari dispatch the events in the following order:

  1. blur
  2. focusout
  3. focus
  4. focusin

IE 11 follows the spec order, i.e.:

  1. focusout
  2. focusin
  3. blur
  4. focus

jQuery follows the order:

  1. focusout
  2. blur
  3. focusin
  4. focus

The fact that IE follows the spec order is a result of blur & focus events being asynchronous there. That's also why we don't shim focusin/focusout in IE via focus/blur as in other browsers as that'd make focusin/focusout asynchronous as well. This was considered in #3123.

As you can notice, the jQuery order doesn't match the spec either, it just fires blur after focusout & focus after focusin, the rest is different. Since all of the modern browsers settled on a specific non-standard events order, should we just stop shimming it in jQuery 4.0 and rely on native behavior in all browsers?

Link to test case

https://jsfiddle.net/66znLhfw/2/

@mgol mgol added this to the 4.0.0 milestone Feb 18, 2019
@dmethvin
Copy link
Member

I agree this would be great to eliminate as special cases. Currently we allow event delegation on "focus" or "blur" events that do not bubble by using the bubbling "focusin" or "focusout". Ideally we'd just make people use "focusin" or "focusout" for those cases.

Although that would be a breaking change it might be worthwhile to require because people would need to look at their code to ensure it's working properly. This should be possible to detect pretty well with Migrate because non-focusable elements (not natively focusable or no tabIndex) shouldn't have focus/blur attached to them. The fix in most cases would simply be to change to "focusin" and "focusout".

@mgol
Copy link
Member Author

mgol commented Feb 19, 2019

@dmethvin I mostly meant dropping our custom focusin/focusout implementation. The fact that focus/blur delegate to focusin/focusout when delegating is defined separately; we have a similar configuration for mouseenter/mouseleave.

Would you want to remove that part as well? This would have to be done for mouseenter/mouseleave as well for consistency, probably ripping out special.delegateType support.

I guess we'll see how much of it survives our event rewrite, though delegateType support doesn't seem to occupy that much space.

@dmethvin
Copy link
Member

Oh true, I agree they're independent decisions. As far as focusin/out are concerned I think it would be fine to use the native implementation and document that all these events currently aren't guaranteed (by the browser) to be in the same relative order.

@timmywil
Copy link
Member

timmywil commented Mar 4, 2019

Leaning towards removing the shim, matching browser behavior, and not guaranteeing execution order.

@mgol
Copy link
Member Author

mgol commented Mar 12, 2019

One note: while the current order is inconsistent between IE & others, what's consistent (& verified by tests) is that focusin fires before focus & focusout before blur. By removing our shim we'll be dropping that guarantee.

I think we should do it but we should realize that we'll drop this guarantee then.

mgol added a commit to mgol/jquery that referenced this issue Apr 17, 2019
Latest versions of all browsers now implement focusin & focusout natively
and they all converged on a common event order so it doesn't make much sense
for us to normalize it to a different order anymore.

Note that it means we no longer guarantee that focusin fires before focus
and focusout before blur.

Fixes jquerygh-4300
@mgol
Copy link
Member Author

mgol commented Apr 17, 2019

PR: #4362

mgol added a commit to mgol/jquery that referenced this issue Apr 17, 2019
Latest versions of all browsers now implement focusin & focusout natively
and they all converged on a common event order so it doesn't make much sense
for us to normalize it to a different order anymore.

Note that it means we no longer guarantee that focusin fires before focus
and focusout before blur.

Fixes jquerygh-4300
mgol added a commit to mgol/jquery that referenced this issue Apr 18, 2019
Latest versions of all browsers now implement focusin & focusout natively
and they all converged on a common event order so it doesn't make much sense
for us to normalize it to a different order anymore.

Note that it means we no longer guarantee that focusin fires before focus
and focusout before blur.

Fixes jquerygh-4300
mgol added a commit to mgol/jquery that referenced this issue Apr 18, 2019
Latest versions of all browsers now implement focusin & focusout natively
and they all converged on a common event order so it doesn't make much sense
for us to normalize it to a different order anymore.

Note that it means we no longer guarantee that focusin fires before focus
and focusout before blur.

Fixes jquerygh-4300
mgol added a commit to mgol/jquery that referenced this issue Apr 23, 2019
Latest versions of all browsers now implement focusin & focusout natively
and they all converged on a common event order so it doesn't make much sense
for us to normalize it to a different order anymore.

Note that it means we no longer guarantee that focusin fires before focus
and focusout before blur.

Fixes jquerygh-4300
mgol added a commit that referenced this issue Apr 29, 2019
Latest versions of all browsers now implement focusin & focusout natively
and they all converged on a common event order so it doesn't make much sense
for us to normalize it to a different order anymore.

Note that it means we no longer guarantee that focusin fires before focus
and focusout before blur.

Fixes gh-4300
Closes gh-4362
@lock lock bot locked as resolved and limited conversation to collaborators Oct 26, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

Successfully merging a pull request may close this issue.

3 participants