53771 - can't open Composer window with Command-Shift-N keybinding

Status: VERIFIED FIXED

(Core :: DOM: UI Events & Focus Handling, defect, P3)

Description

[Kathleen :Brade]

On my Mac build, I can't open a new composer window using Command-Shift-N.
The shortcut does appear correctly in the File | New menu.

My Linux build does react appropriately to Control-Shift-N.

Comment 1

[sairuh (rarely reading bugmail)]

i see this too using 2000.09.22.08 opt com on Mac OS 9.0.

definitely mac only --cannot repro on winNT either.

Comment 2

[Dan Rosen]

looking, but i'm having a harder time finding this than the earlier "moodifiers"
typo. working on it...

Comment 3

[Dan Rosen]

it's a little strange that this accelerator key exists in the navigator window
but not in the mail or address book windows... probably irrelevant, though.

Comment 4

[Dan Rosen]

found it. xpfe/communicator/resources/content/utilityOverlay.xul:40, which
nothing seems to be wrong with. my wild guess is that we managed to break the
observes="..." syntax on the mac with our new keybinding stuff. hopefully
another simple fix in nsXBLWindowKeyListener. reassigning to saari, who might
have a better idea...

Comment 5

[saari (gone)]

We know what the problem is, event key upper vs. lower case differences between
Mac and other platforms. We just have to fix it/decide what the "right" fix is.
That is probably whatever 4.x did or IE does (which may be different to being with).

Comment 6

[Peter Trudelle]

rtm need info, since removing the key from the menu would probably be higher risk.

Comment 7

[saari (gone)]

Attaching patch here, but I need to test it with IME. So, ftang, blee,
sfraser... if any of you could lend me a hand in testing this on a Mac with a
language kit installed, I would appreciate it!

Index: nsMacEventHandler.cpp
===================================================================
RCS file: /m/pub/mozilla/widget/src/mac/nsMacEventHandler.cpp,v
retrieving revision 1.104
diff -w -u -2 -r1.104 nsMacEventHandler.cpp
--- nsMacEventHandler.cpp	2000/09/22 06:03:06	1.104
+++ nsMacEventHandler.cpp	2000/10/05 23:28:05
@@ -802,4 +802,7 @@

		aKeyEvent.keyCode	= 0;

		aKeyEvent.charCode = ConvertKeyEventToUnicode(aOSEvent);
+
		if(aKeyEvent.isShift && aKeyEvent.charCode <= 'Z' && aKeyEvent.charCode >= 'A') {
+
		  aKeyEvent.charCode -= 32;
+
		}

		NS_ASSERTION(0 != aKeyEvent.charCode, "nsMacEventHandler::InitializeKeyEvent:
ConvertKeyEventToUnicode returned 0.");

	} // else for if ( aKeyEvent.isControl )

Comment 8

[David Hyatt]

Patch seems backwards to me.  Shouldn't it be asking if the charcode is between 
"a" and "z" and not between "A" and "Z"?

Comment 9

[saari (gone)]

Doh, that is what I get for not trying the cleaned up patch. Try this...

Index: nsMacEventHandler.cpp
===================================================================
RCS file: /m/pub/mozilla/widget/src/mac/nsMacEventHandler.cpp,v
retrieving revision 1.104
diff -w -u -2 -r1.104 nsMacEventHandler.cpp
--- nsMacEventHandler.cpp	2000/09/22 06:03:06	1.104
+++ nsMacEventHandler.cpp	2000/10/05 23:52:38
@@ -802,4 +802,7 @@

		aKeyEvent.keyCode	= 0;

		aKeyEvent.charCode = ConvertKeyEventToUnicode(aOSEvent);
+
		if(aKeyEvent.isShift && aKeyEvent.charCode <= 'z' && aKeyEvent.charCode >= 'a') {
+
		  aKeyEvent.charCode -= 32;
+
		}

		NS_ASSERTION(0 != aKeyEvent.charCode, "nsMacEventHandler::InitializeKeyEvent:
ConvertKeyEventToUnicode returned 0.");

	} // else for if ( aKeyEvent.isControl )

Comment 10

[Simon Fraser [no longer active]]

Does this patch only affect shortcut keys (i.e. with command down), or all 
typing? Also, this seems the wrong place to be doing this converions. Why not 
just a case insensitive string compare somewhere in the XUL code?

Comment 11

[saari (gone)]

XUL code has nothing to do with it this is XBL now, and even so, it is more
proper to make sure all the platforms are sending the same thing in the events.

But I should make this apply to all key events.

Does anyone know if this breaks IME? This is my main concern.

Comment 12

[Simon Fraser [no longer active]]

We tested IME and it looks fine. r=sfraser

Comment 13

[David Hyatt]

a=hyatt

Comment 14

[David Hyatt]

Ready for PDT.

Comment 15

[Phil Peterson]

rtm++

Comment 16

[saari (gone)]

Fix committed on branch and trunk

Comment 17

[Kathleen :Brade]

*** Bug 47677 has been marked as a duplicate of this bug. ***

Comment 18

[sairuh (rarely reading bugmail)]

vrfy fixed on Mac OS 9.0 using opt comm branch bits, 2000.10.09.10-n6.

needs trunk vrf'tion.

Comment 19

[Asa Dotzler [:asa]]

Verified Fixed on trunk builds
mac 101804 Mac OS9
Setting bug to Verified and removing vtrunk keyword