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

[Joseph Scheuhammer]

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:

• Use the test file in "​.../dijit/tests/test_Menu.html"
• Put focus on, for example, the text box containing the string "top-left"
• Press cntl+space

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...

[Joseph Scheuhammer]

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?

[davidb]

Is this related to ​mozbug459744 ?

[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().

[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.

[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.

[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.

[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.

[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.

[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.

[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).

[bill]

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

[Becky Gibson]

(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