Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#17668 closed defect (fixed)

Dijit focus manager sequence problem in Chrome

Reported by: Ed Hager Owned by: Bryan Forbes <bryan@…>
Priority: undecided Milestone: 1.7.6
Component: Dijit Version: 1.9.0
Keywords: Cc:
Blocked By: Blocking:

Description (last modified by Ed Hager)

The dijit test dijit.tests.robot.BgIframe? is failing in Chrome due to a defect in dijit/focus. The test is trying to select a menu item from a drop-down button's popup but the popup is not visible when the test tries to focus on the menu item.

I have a test page that demonstrates the problem. It can be accessed here: http://jsbin.com/ewiXEJA/6. I will also attach an HTML file to this ticket.

In the test page:

  1. Click on the date text box to cause the calendar drop-down to appear.
  2. Click on one of the "Edit" buttons.
  3. The drop-down button's popup will appear and then immediately close.

The defect was introduced in commit 9965406f7184e8cee4467efc893ca5d85637b2e1.

The problem is the "clear active widgets timer", https://github.com/dojo/dijit/blob/1.9.0/focus.js#L187-L190, is firing in Chrome before the "focusin" event occurs. In Safari and Firefox, the focusin event fires, dijit/focus#_onTouchNode is called and the timer is cancelled. Changing the timer's delay to 500ms causes the test page to work properly in Chrome.

Attachments (2)

focus_manager_defect.html (1.3 KB) - added by Ed Hager 5 years ago.
focus_manager_defect.2.html (1.6 KB) - added by Bryan Forbes 5 years ago.

Download all attachments as: .zip

Change History (21)

Changed 5 years ago by Ed Hager

Attachment: focus_manager_defect.html added

comment:1 Changed 5 years ago by Ed Hager

Description: modified (diff)

comment:2 Changed 5 years ago by bill

OK thanks... you mean the focus event on the MenuItem? (in the drop down menu)? Or on the Edit button itself?

comment:3 Changed 5 years ago by Ed Hager

On the edit button.

Here is what I think is happening... The focusout event fires due to the date text box losing focus. _onBlurNode is called on the focus manager which creates the timer. The timer fires before the edit button's focusin event fires. When the timer fires, _setStack is called which causes the edit button's popup to be closed.

One question I had is why does the error present itself when you move from the date text box to an edit button rather than between the two edit buttons. That is why my test page has two edit buttons.

comment:4 Changed 5 years ago by bill

That's indeed a good question, and I'm not sure of the answer. In both cases, before clicking the Edit button, the respective drop down has focus, right? As opposed to the anchor node that opened the drop down (DropDownButton? or DateTextBox?).

comment:5 Changed 5 years ago by Ed Hager

Correct. The data text box focuses on today's date in the calendar, When you click on an Edit button, you can hover over a menu item and it will receive focus. In that case when you click on the second Edit button, the focus manager's old stack will contain a MenuItem?, a DropDownMenu? and the DropDownButton?.

comment:6 Changed 5 years ago by bill

OK. Well 500ms seems like a long delay. I'm just thinking that if I open a drop down and then click a blank area of the screen (to close the drop down), the UI would feel sluggish if it took 1/2 a second. Could it be something less, like 50ms?

comment:7 Changed 5 years ago by Ed Hager

A bit more info...

Using the touchpad on my MacBook? Pro, if I push down on the pad and make it click to do the mouse clicks, then I see the error. But if I tap the pad to do the mouse clicks, then everything works fine.

Changed 5 years ago by Bryan Forbes

Attachment: focus_manager_defect.2.html added

comment:8 Changed 5 years ago by Bryan Forbes

After extensive testing and tracing, it seems that Chrome fires focusout and blur events for focused nodes that get hidden. I have attached another test case which opens the date drop down, focuses it, and then hides it using CSS after 2 seconds. I have also attached a focusout handler to the body to show that Chrome does indeed fire that event.

This means that when the mousedown event happens on the drop down button, it is firing a focusout event that doesn't occur in other browsers. The focusout event happens after the timeout from the mousedown handler in focus.js sets focus._justMouseDowned to false. Then the focusout handler calls focus._onBlurNode() which sets the focus stack to [] in the timeout for clearing the active widgets in focus._onBlurNode().

I and Ed have found that increasing the timeout of the handler that sets focus._justMouseDowned to false in focus.js to 13 allows enough time for the focusout handler to fire before setting focus._justMouseDowned to false and will allow focus._onBlurNode() to short-circuit. I will be submitting a pull request with the change.

comment:9 Changed 5 years ago by bill

After extensive testing and tracing, it seems that Chrome fires focusout and blur events for focused nodes that get hidden.

Hmm, hiding a focused node is bad news in general; [older] IE had various serious problems with that. Why doesn't the mousedown on the DropDownButton? shift focus from the Calendar drop down to the DropDownButton??

I can imagine an issue when clicking on the down arrow of another DateTextBox?, since the arrow itself isn't focusable, but I'm confused why there's an issue when clicking on a DropDownButton?.

comment:10 Changed 5 years ago by Bryan Forbes

Hiding a focused node was not the problem in old IE; the problem was calling node.focus() on a hidden node would throw an error.

The mousedown will generate a focus shift (the default action of mousedown), however event handlers run before default actions. Since the mousedown handler closes the calendar drop down, the focusout event generated by hiding the node is fired before the browser has a chance to take care of moving the focus itself in the default action of mousedown. This means that it doesn't matter what control is clicked: if a popup is open and it has focus, clicking another control that opens a drop down will immediately close the new drop down (and yes, I just tested it with a second DateTextBox control on the page as well). Note, however, that this doesn't happen when you start with one of the the DropDownButton menus open because the focus is not in the menu, but on the button.

comment:11 Changed 5 years ago by Bryan Forbes

We have run the BgIframe and FocusManager robot tests on Chrome, Firefox, IE8, IE9, IE10, and IE11 and all pass.

comment:12 Changed 5 years ago by bill

Actually, IE also has problems hiding a focused node, but they are subtle.

Anyway, your fix seems good to me; go ahead and check it in. I guess we don't need a test case since BgIframe? is already failing.

Thanks for tracking this down.

comment:13 Changed 5 years ago by bill

Some notes for the record:

(1) Dijit's focus manager mainly watches focusin and mousedown (and touchstart/pointerdown) events. The focusout listener is just to handle the corner case where the user tabs out of the browser window into the address bar, or something like that.

(2) Therefore, my comment:6 about sluggishness responding to a mousedown event (to close a dropdown) didn't make sense. The mousedown will instantly close the drop down. I mention this partly because in today's meeting some people were concerned whether 13ms is enough for slow machines.

(3) There are two pieces of code to ignore focusout events. One is here:

var foh = on(body, 'focusout', function(evt){
	// IE9+ has a problem where focusout events come after the corresponding focusin event.  At least
	// when moving focus from the Editor's <iframe> to a normal DOMNode.
	if((new Date()).getTime() < lastFocusin + 100){
		return;
	}

	_this._onBlurNode(effectiveNode || evt.target);
});

The other is in _onBlurNode():

_onBlurNode: function(/*DomNode*/ node){
	...

	// If the blur event isn't followed by a focus event, it means the user clicked on something unfocusable,
	// so clear focus.
	if(this._clearFocusTimer){
		clearTimeout(this._clearFocusTimer);
	}
	this._clearFocusTimer = setTimeout(lang.hitch(this, function(){
		this.set("prevNode", this.curNode);
		this.set("curNode", null);
	}), 0);

	if(this._justMouseDowned){
		// the mouse down caused a new widget to be marked as active; this blur event
		// is coming late, so ignore it.
		return;
	}

I'm not sure why those two pieces of code are in separate places.

More importantly, it seems wrong that _onBlurNode still sets the _clearFocusTimer even when _justMouseDowned == true. Probably that's an error.

Edit on May 5, 2014:

On second thought, the current code makes sense to me. If there's a blur event on node A and a touch event on node B (but no focus event), then it makes sense to set curNode to null, but to set the widget stack to be on node B. In other words, keeping track of the currently focused node and the widget stack are really separate things.

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

comment:14 Changed 5 years ago by Bryan Forbes <bryan@…>

Owner: set to Bryan Forbes <bryan@…>
Resolution: fixed
Status: newclosed

In 7cb2066cebba23d9e08af5f2e0210203bde3436a/dijit:

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

comment:15 Changed 5 years ago by Bryan Forbes <bryan@…>

In 54aeab18b482e1294aebbde2eb17b13ae7af066f/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 5 years ago by Bryan Forbes <bryan@…>

In 321091aa2558e4258a22a70bbae7a54a830a0a3e/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

Milestone: tbd1.8.7

comment:18 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:19 Changed 5 years ago by bill

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