448434 - Some shifted keys return an incorrect keycode

Status: RESOLVED FIXED

(Core :: Widget: Cocoa, defect, P3)

Description

[Greg K.]

As reported in bug 44259, comment 50, some shifted key combinations still incorrectly return a zero keycode on Mac. The reported combinations are as follows.

shift-- (_)
shift-` (~)
shift-\ (|)
shift-, (<)
shift-. (>)
shift-/ (?)
shift-; (:)

Comment 1

[Steven Michaud [:smichaud] (Retired)]

I've confirmed this (using attachment 246186 [details], originally from bug
44250), on FF3.0.1, a recent Firefox trunk (1.9.1-branch) nightly, and
a recent Camino 1.9.0-branch nightly.

Comment 2

[Peter Phillips]

I'm seeing this on FireFox 3.6.2 under Mac OSX.

Comment 3

[Conrad Damon]

It'll be part of FF until they fix it. The workaround is to wait for keypress (and translate charCode to keyCode if necessary) or keyup.

It seems odd that the fix for bug 44259 omitted seven characters; you'd think it wouldn't be hard to complete the fix.

Comment 4

[Jan Wolter]

Attachment: A completely untested path for this bug.

I don't even have a working Mac right now, so I can't compile Mozilla or test it, but I've been waiting eleven years for this bug to be fixed, and it's not that hard.  I'm reasonably confident that this will make the keycodes on keyup and keydown events on the Macintosh match those on Windows and Linux for US keyboards. Since the code as currently written maps character codes to keycodes, I'm not at all sure that that is the right approach for international keyboards, but, if not, that is probably a different bug. This just fixes the fact that the current code returns zero for many shifted symbol characters, instead of returning the same code as the unshifted key does, and fixes the incorrect keypad plus code being returned for the keyboard plus key.

Comment 5

[Chris Lawson (gone)]

I'm pretty sure kreeger isn't actively working on this code any more; you might want to find someone else to review it.

Josh or Steven, do you know of anyone who'd be a good r? for this?

Comment 6

[Steven Michaud [:smichaud] (Retired)]

I can review this.  I'll do it in sync with my reviews of Masayuki's patches for bug 677252.  I hope to get to them next week.

Comment 7

[Steven Michaud [:smichaud] (Retired)]

The testcase I used in comment #1 is still available.  I used it to confirm that Masayuki's patches for bug 677252 do indeed fix this bug.

Here's a tryserver build made with those patches:

https://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-5dfc229cc591/try-macosx64/firefox-10.0a1.en-US.mac.dmg

Please test with it and let us know your results.

Comment 8

[Steven Michaud [:smichaud] (Retired)]

Comment on attachment 553319 [details] [diff] [review]
A completely untested path for this bug.

In light of what I said in comment #7, this is unneeded.

Comment 9

[Wayne Mery (:wsmwk)]

per comment 7