Opened 9 years ago

Closed 9 years ago

#11369 closed defect (fixed)

Dialog: takes 2 clicks to focus content

Reported by: ben hockey Owned by: bill
Priority: high Milestone: 1.6
Component: Dijit Version: 1.5.0b2
Keywords: Cc: Becky Gibson
Blocked By: Blocking:


to focus the tab container in the dialog, you have to click on it twice. you can see this by the console.logs which are happening in response to onFocus and onBlur. in contrast, if you click on the tab container which is not in the dialog, you will see that it receives focus on the first click.

i noticed this problem while trying to set a listener for key events when one of my containers in a dialog receives focus. onFocus causes the listener to be active and onBlur causes it to stop listening. unfortunately, with the way this is, the user has to click twice for the handler to be active.

the attached test case is a cut and paste of test_Dialog.html so all the paths are set the same.

Attachments (1)

test_Dialog.html (5.9 KB) - added by ben hockey 9 years ago.

Download all attachments as: .zip

Change History (9)

Changed 9 years ago by ben hockey

Attachment: test_Dialog.html added

comment:1 Changed 9 years ago by bill

I'm seeing two "focus" messages appear in the console, but not having to click twice. Where are you clicking, on the tab button? Which browser do you see the failure on?

comment:2 Changed 9 years ago by ben hockey

ok... it might be kind of confusing since i left in a 'focus' message from the original test. i probably should have removed that one. i should have removed lines 35-37 for the sake of clarity.

i'm clicking on the content of the tab inside the dialog (opened by clicking the "Show TabContainer? Dialog" button). in FF 3.6.4 the first time i click on the content in the tab i see a focus message ('focused') followed by an immediate blur message ('blurred'). the 2nd click actually manages to hold the focus and i just see the 'focused' message.

i also added a handler to the tab container that is not in the dialog. the messages from that handler are 'outside focused' and 'outside blurred'. these were just included to show the behavior i'm expecting to see for the tab container inside the dialog.

comment:3 Changed 9 years ago by bill

I see, that's a weird one. To put it another way, the problem is that clicking the TabContainer puts focus (keyboard focus with dotted black border) on the Dialog itself.

comment:4 Changed 9 years ago by ben hockey

yes - that's what i'm seeing. i tried to follow what was happening with dijit's focus manager but i didn't have the patience at the time to find the cause of the problem.

it seemed like dijit._activeStack was being calculated incorrectly due to "a difference of opinions" between focusListener and mousedownListener.

as i said, i didn't have the patience to work through it at the time and was hoping someone more familiar with that code might have a better perspective on what's wrong.

comment:5 Changed 9 years ago by bill

The stuff with _activeStack/focusListener/mouseDownListener is all downstream from the actual problem, which is that the Dialog is getting focus on mousedown. That's happening even when the focus manager is disabled.

comment:6 Changed 9 years ago by bill

Milestone: tbd1.6
Owner: set to bill
Status: newassigned

Looks like the just removing tabindex="-1" from Dialog's template fixes the problem. The -1 is actually causing the Dialog to be focusable by clicking, the opposite of the effect we wanted.

comment:7 Changed 9 years ago by bill

Cc: Becky Gibson added
Summary: dijit.Dialog takes 2 clicks to focus contentDialog: takes 2 clicks to focus content

Becky, you added the tabindex="-1" in [12144]. Looks like it can be removed but do you see any issues?

Even after removing that tabindex="-1" setting, In test_Dialog.html if I click "show unmovable" (which is a dialog w/no focusable items), I can tell from the console message "focused on cantmove" that the Dialog itself gets focus. Tested on FF3.6/mac, Chrome5/mac, and IE6.

comment:8 Changed 9 years ago by bill

Resolution: fixed
Status: assignedclosed

(In [22490]) Remove tabIndex setting from naturally non-focusable nodes (<div>, <table>, <ul>, etc.). Originally the tabIndex setting was needed for FF2 (and maybe FF3.0) when setting the (aria) role, but it's not needed anymore, and sometimes is harmful.

Didn't remove tabIndex setting for nodes that are eventually [programatically] focused: MenuItem, CheckedMenuItem, MenuBarItem, TreeNode. That's because tabindex is used as a cue to CssStateMixin to setup focus handlers on those nodes.

Fixes #11369, refs #2718 !strict.

Note: See TracTickets for help on using tickets.