Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#16970 closed defect (fixed)

[patch] [cla] on(node, "focusin", ...) calls listener too late on IE9+

Reported by: bill Owned by: Kris Zyp
Priority: undecided Milestone: 1.8.6
Component: Events Version: 1.8.3
Keywords: Cc:
Blocked By: Blocking: #16968

Description

on.js has code for map "focusin" and "focusout" types to "focus" and "blur" for browsers that don't support "focusin" and "focusout" natively:

if(has("dom-addeventlistener")){
        // normalize focusin and focusout
        captures = {
                focusin: "focus",
                focusout: "blur"
        };
        ...

However, this code runs for IE9+, firefox, and chrome, whereas it really should only run for firefox.

In particular this leads to problems on IE where on(node, "focusin", listener) calls the listener asynchronously. I hit it after [30957] and I guess it led to many of the fixes/workaround in #16926, but #16968 is trickier so perhaps we should fix the real bug rather than working around it.

Attachments (1)

focusin.patch (1.7 KB) - added by bill 6 years ago.
simple patch to stop using workaround code for IE9+, and just use it for firefox (where it's needed) and webkit (where it doesn't hurt)

Download all attachments as: .zip

Change History (7)

comment:1 Changed 6 years ago by bill

Blocking: 16968 added

(In #16968) It turns out this started with [30957] with the root cause of #16970.

comment:2 Changed 6 years ago by bill

Milestone: tbd1.9
Summary: on(node, "focusin", ...) on IE9+ and chrome[patch] [cla] on(node, "focusin", ...) calls listener too late on IE9+

I'm attaching a patch, does it look OK? True feature testing is quite difficult since webkit supports focusin but there's no clue like element.onfocusin being defined. The true feature test (as listed in http://stackoverflow.com/questions/7337670/how-to-detect-focusin-support) requires attaching an element to the document and actually focusing it, which seems a bit crazy.

For now I just check if element.attachEvent is defined. It's similar to the current code which checks whether element.addEventListener is defined, except that it does the right thing on IE9+ by listening to focusin instead of focus. (To put it another way, you shouldn't assume that addEventListener("focusin", ...) doesn't work just because addEventListener() is defined.)

Last edited 6 years ago by bill (previous) (diff)

Changed 6 years ago by bill

Attachment: focusin.patch added

simple patch to stop using workaround code for IE9+, and just use it for firefox (where it's needed) and webkit (where it doesn't hurt)

comment:3 Changed 6 years ago by Kris Zyp

This looks like a good patch, go ahead and commit it.

comment:4 Changed 6 years ago by bill

Resolution: fixed
Status: newclosed

In [31152]:

Use IE's native focusin/focusout support for IE9+ too, avoiding asynchronous focus notification, fixes #16968, #16970, and refs #16926, #16972 !strict.

comment:5 Changed 6 years ago by mahays0 <mahays0@…>

In b9ab7f580c8a123ff07ebd9a3b0695e24cf5c5ba/dojo:

Error: Processor CommitTicketReference failed
Unsupported version control system "git": Can't find an appropriate component, maybe the corresponding plugin was not enabled? 

comment:6 Changed 6 years ago by bill

Milestone: 1.91.8.6

Note: this change is superseded by #17599.

Note: See TracTickets for help on using tickets.