Opened 8 years ago

Closed 7 years ago

Last modified 5 years ago

#13341 closed defect (fixed)

_onFocus is called before _onMouseDown

Reported by: siqi Owned by: bill
Priority: low Milestone: 1.7.6
Component: Dijit Version: 1.7.0b1
Keywords: Cc: zhongsq@…
Blocked By: Blocking:

Description (last modified by bill)

Open dijit/tests/form/test_CheckBox.html, add following code into dojo.addOnLoad:

dojo.connect(dijit.byId("cb1"), "_onMouseDown", function(){console.log("mouseDown");});
dojo.connect(dijit.byId("cb1"), "_onFocus", function(){console.log("focus");});

Click on the checkbox, it prints "focus" thand "mouseDown" in the console.

Is it what we expected? I think "_onMouseDown" should be called before "_onFocus", in the same order as the native events 'mousedown' -> 'focus' -> 'mouseup' -> 'click'.

Change History (21)

comment:1 Changed 8 years ago by bill

Priority: highlow
severity: criticalminor

This is expected, due to how the focus manager is written: it catches the onmousedown event before cb1 does, and then calls cb1._onFocus(). Note that _onFocus is misnamed as it doesn't correspond exactly to DOM focus events, but too late to change the name now.

You could always connect to "onfocus" on cb1.focusNode to get the behavior you want.

comment:2 in reply to:  1 Changed 8 years ago by siqi

Replying to bill:

This is expected, due to how the focus manager is written: it catches the onmousedown event before cb1 does, and then calls cb1._onFocus(). Note that _onFocus is misnamed as it doesn't correspond exactly to DOM focus events, but too late to change the name now.

You could always connect to "onfocus" on cb1.focusNode to get the behavior you want.

Oh yes, you're right. The work around works well. :-)

comment:3 Changed 8 years ago by bill

Component: Dijit - FormDijit
Resolution: wontfix
Status: newclosed

OK great. We could make _onFocus() occur after _onMouseDown() with a setTimeout(), but we try to avoid setTimeout() whenever possible. Other than that I don't know a way to change the order of those events. So I'm going to mark this as wontfix for now, directing people to use that workaround.

comment:4 in reply to:  3 Changed 8 years ago by siqi

Replying to bill:

OK great. We could make _onFocus() occur after _onMouseDown() with a setTimeout(), but we try to avoid setTimeout() whenever possible. Other than that I don't know a way to change the order of those events. So I'm going to mark this as wontfix for now, directing people to use that workaround.

Exactly, setTimeout() is always not what we want....:-)

comment:5 Changed 8 years ago by bill

Description: modified (diff)

comment:6 Changed 8 years ago by bill

#14838 is a duplicate of this ticket.

comment:7 Changed 8 years ago by bill

#15132 is a duplicate of this ticket.

comment:8 Changed 8 years ago by bill

Milestone: tbdfuture
Resolution: wontfix
Status: closedreopened

I suppose if we did have a setTimeout(), it could be to trigger the so-called _onFocus() event when there was a mousedown event which wasn't followed immediately by a focus event (such as the case of clicking the non-focusable up/down arrows on a spinner). I.e. setup a timer on mousedown, but clear it onfocus.

Perhaps since this has come up a number of times I should do that. I'll reopen this to consider.

Alternately, perhaps the focus manager should just ignore mouse events, and clicking the up/down arrows on a spinner shouldn't cause it to focus.

PS: Need to think about _onBlur() events too though. Ideally clicking the arrows on a focused spinner shouldn't cause an _onBlur() call, since that would make the value reformat from 12345 to 12,345. Or worse yet, an _onBlur() followed after 0ms by another _onFocus().

Also consider stuff like iOS where the mousedown, mouseup, and click events all occur together. We wouldn't want an _onFocus() call to occur after the click event, as it might lead to problems like an onclick listener opening a TooltipDialog and then the Button._onFocus() call immediately closing that TooltipDialog.

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

comment:9 Changed 8 years ago by Douglas Hays

In [28290]:

Refs #14968, #13341. Move _onMouseDown processing to _onFocus(by=mouse) to better align with the focus manager's current behavior of invoking _onFocus before onfocus and before onmousedown.

comment:10 Changed 7 years ago by bill

Milestone: future1.9
Owner: changed from Douglas Hays to bill
Status: reopenedassigned

comment:11 Changed 7 years ago by bill

Resolution: fixed
Status: assignedclosed

In [30957]:

Change focus manager to use dojo/on rather than addEventListener() and attachEvent() on older IE's. This makes the behavior consistent across browsers (as to listening on capture phase vs. bubbling phase), and as a side effect fixes #13341 !strict.

comment:12 Changed 7 years ago by bill

In [30994]:

Fix regression from [30957]: evt.target is the normalized way to reference the event target, not evt.srcElement. Fixes exception on firefox. Refs #13341 !strict.

comment:13 Changed 7 years ago by bill

In [31012]:

Need to listen for touchstart in addition to mousedown, or otherwise clicking a blank area of the screen won't close popups on iOS. Fixes regression from [30957], refs #13341 !strict. Behavior still not perfect: clicking a blank area of the screen closes the popup but then focuses the associated input (at least on test_DateTextBox.html), unwantedly bringing up the keyboard.

comment:14 Changed 7 years ago by bill

In [31237]:

update test to not rely on dijit/focus, and add TODO about apparently problem with dijit/focus code, refs #13341 !strict

comment:15 Changed 6 years ago by Bill Keese <bill@…>

In 76216d6d829f011a87ccd11ee12d3a862d6e2e88/dijit:

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

comment:16 Changed 6 years ago by Bill Keese <bill@…>

In d5eef52f83e77b9d501ce0309b4f4d69b64c6a04/dijit:

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

comment:17 Changed 5 years ago by bill

[30957] backported to 1.8 in e2b992b720df9936af690e08de1a76c299f38613.

comment:18 Changed 5 years ago by Bill Keese <bill@…>

In 809130120f849c06f71ceadc8edeb3e2edd9db1b/dijit:

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

comment:19 Changed 5 years ago by Bill Keese <bill@…>

In 9de5f5c7f5df191596bab83a18e8871eeba1158e/dijit:

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

comment:20 Changed 5 years ago by Bill Keese <bill@…>

In b705fc4a494f78f9519a4df8979b94ac1cbb7fb5/dijit:

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

comment:21 Changed 5 years ago by bill

Milestone: 1.91.7.6
Note: See TracTickets for help on using tickets.