Opened 11 years ago

Closed 11 years ago

#8406 closed defect (fixed)

[patch] [cla] Context menu: keyboard invocation of context menu does not work for FF3 on Mac

Reported by: Joseph Scheuhammer Owned by: Becky Gibson
Priority: high Milestone: 1.3
Component: Accessibility Version: 1.2.3
Keywords: context menu, keyboard, a11y, Mac, FF3 Cc: Becky Gibson, bill, Douglas Hays, davidb
Blocked By: Blocking:

Description

Documentation lists cntl+space as the keystroke for invoking a context menu for FF on Mac. This works fine in FF2. But, in FF3, the behaviour is:

  1. The context menu appears,
  2. The first menu item in the menu is selected and activated immediately,
  3. The menu closes.

To duplicate:

Note that there is no problem invoking the context menu with the mouse using either right-click or cntl+click.

Setting the milestone for 1.3, but am not sure if this qualifies as a show stopper. Trying to diagnose...

Attachments (2)

8406.patch (565 bytes) - added by Joseph Scheuhammer 11 years ago.
8406a.patch (9.1 KB) - added by Joseph Scheuhammer 11 years ago.

Download all attachments as: .zip

Change History (17)

comment:1 Changed 11 years ago by Joseph Scheuhammer

Cc: Douglas Hays added

I've been using grep to find out where control+space is handled. I'm not finding it. I'm beginning to suspect it has something to do with the way FF handles that key combo, since it is the only browser that does (on Mac). Anyone recall/know where to look?

comment:2 Changed 11 years ago by Becky Gibson

severity: normalmajor

comment:3 Changed 11 years ago by davidb

Is this related to mozbug459744 ?

comment:4 Changed 11 years ago by davidb

Cc: davidb added

comment:5 Changed 11 years ago by Joseph Scheuhammer

Here's a clue: HTML 5 defines a new event 'oncontextmenu'. Some of HTML 5 is implemented in FF, but I'm not sure if this one is.

Nonetheless, Menu.js makes use of that event (line 307 in my copy in function dijit.Menu.bindDomNode()):

    dojo.connect(cn, (this.leftClickToOpen)?"onclick":"oncontextmenu", this, "_openMyself"),

Hypothesis: on Mac, FF reacts to the control+space key combo and generates an 'oncontextmenu' event, that dijit.Menu, in turn, tries to handle. If that's right, the bug may reside somewhere in _openMyself().

comment:6 Changed 11 years ago by Joseph Scheuhammer

The best bet is that this is an FF3 issue, but there is a one-line workaround.

Rationale: placing a break-point at dijit.Menu.onItemClick(), and doing a control+space key combo results in the following stack:

  • onItemClick() // djit.Menu.onItemClick()
  • _onClick() // dijit.MenuItem._onClick()
  • (?)() // dojo.hitch(dijit_MenuItem_0, "_onClick")
  • (?)() // anon function at line 801 in dijit._Widget.connect()
  • method() // dojo.hitch(dijit_MenuItem_0, method)

where Line 801 of dijit._Widget.js is:

800 dco("onkeyup", this, function(e){
801       if(e.keyCode == d.keys.SPACE){ return m(e); }
802 })

This is proof that the SPACE keyup event is posted by FF3 in addition to posting a contextmenu event.

The patch, "8406.patch" adds a check to dijit.Menu.onItemClick() to see if the "click" is caused by a control+space. If so, onItemClick() does nothing. If it is an unmodified space, onItemClick() functions as before.

I'm not sure if this is the best place to make the check, but it appears to be the place where the event is handled. Becky, what's your opinion?

The patch was tested using FF2/FF3 Mac; FF3/IE8RC1 WinXP.

Changed 11 years ago by Joseph Scheuhammer

Attachment: 8406.patch added

comment:7 Changed 11 years ago by bill

Oh it sounds like that code in _Widget.js that you quoted above shouldn't do anything if the control key (or probably shift or other meta keys) were depressed on the onkeyup event.

Maybe

if(e.keyCode == d.keys.SPACE && !e.ctrlKey && !e.shiftKey && !e.altKey)){ return m(e); }

Likewise for the onkeydown and onkeypress handlers around that area too.

comment:8 in reply to:  7 Changed 11 years ago by Joseph Scheuhammer

Replying to bill:

Oh it sounds like that code in _Widget.js that you quoted above shouldn't do anything if the control key (or probably shift or other meta keys) were depressed on the onkeyup event. ... Likewise for the onkeydown and onkeypress handlers around that area too.

Are you sure? This is, after all, an FF3/Mac bug. There is no problem in the case of FF2/Mac.

I imagine a lot of code funnels through _Widget.js, so I'm not confident that such a central change wouldn't have bad side effects.

On the other hand, you know the code far better than I do. If you think this is where the change belongs, I'm more than happy to put it there.

Let me know.

comment:9 Changed 11 years ago by bill

Well, the code in question in _Widget.js is just for handling "keyboard click" events, and I think that only a plain SPACE or ENTER should be treated as a keyboard click event? If that's the case then the change belongs there, but try it out to make sure that it both fixes the problem in this ticket and doesn't break keyboard click handling.

See also #8414; I think it's related.

comment:10 in reply to:  9 ; Changed 11 years ago by Joseph Scheuhammer

Replying to bill:

Well, the code in question in _Widget.js is just for handling "keyboard click" events, ...

Yes, the code in question is dealing with 'ondijitclick'. For the tests in ".../dijit/tests/ondijitclick.html", each simulated key event explicitly clears all modifiers. (Aside: another test would have been to have one or more modifiers set, and expect failure).

I'll modify _Widget.js, make sure it solves this ticket, and run the tests.

comment:11 in reply to:  10 Changed 11 years ago by davidb

Replying to clown:

Replying to bill: (Aside: another test would have been to have one or more modifiers set, and expect failure).

I agree, please add those tests and fix if/as needed.

comment:12 Changed 11 years ago by Joseph Scheuhammer

Attaching "8406a.patch" that modifies _Widget.js, specifically dijit.connect() where the event is "ondijitclick". For SPACE and ENTER key events, a check is made for any modifiers (control, alt, shift, or meta). The keystroke is ignored when so modified.

Also, added unit tests for modifier keys -- see ondijitclick.html.

The patch was tested in FF2 and FF3 (Mac), and FF2, FF3, and IE8RC1 (WinXP).

Changed 11 years ago by Joseph Scheuhammer

Attachment: 8406a.patch added

comment:13 Changed 11 years ago by Joseph Scheuhammer

Summary: Context menu: keyboard invocation of context menu does not work for FF3 on Mac[patch] [cla] Context menu: keyboard invocation of context menu does not work for FF3 on Mac

comment:14 Changed 11 years ago by bill

Looks good to me (ie, the code looks good, I haven't run the tests).

comment:15 Changed 11 years ago by Becky Gibson

Resolution: fixed
Status: newclosed

(In [16458]) fixes #8406 commit for clown (ccla on file) Update ondijitclick to ignore modifier keys that are pressed with enter or space. Reviewed by Bill, tested by Becky & Clown. This fixes the ticket problem but brings up an issue that the use of ondijitclick by widgets is inconsistent. Not sure how big an issue that it but, with this patch it makes for inconsistent behavior between widgets when a modifier key is pressed with space or enter. !strict

Note: See TracTickets for help on using tickets.