432632 - [10.5] Cmd+? (in shifted position) doesn't work on certain keyboard layouts

Status: RESOLVED FIXED

(Core :: Widget: Cocoa, defect)

Description

[Stefan [:stefanh]]

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; sv-SE; rv:1.9pre) Gecko/2008050604 Minefield/3.0pre

Pressing Cmd+Shift++ should generate Cmd+? (and ignore the shift key) on a sv-SE keyboard, but it appears that it doesn't work.

If I try to open Help using a sv-SE keyboard, the help menu just flashes, nothing else happens. However, pressing Cmd+Shift+- opens the help placeholder page in a new tab.

Not sure how this should be handled, should the locales (sv-SE, fi etc etc) try to work around this in their .dtd's?

Comment 1

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(sickish, maybe slower respose)]

See bug 306585 comment 15 and bug 306585 comment 36.

This may be a serious bug of MacOS X 10.5.

Comment 2

[Axel Hecht [:Pike]]

I'm seeing this on 10.4.11 and a german keyboard on an en-US build.

Cmd+Shift+ß, which has ? on the shifted position, flashes the help menu item, but doesn't open a window. Cmd+Shift+- does nothing, but I don't know if that matters for a German keyboard. Though it seems to be on the same physical key that holds the '?' in en-US.

So, is the fact that '-' opens help a 10.5 bug?

That doesn't affect that '?' does not open the help window, though, right?

Comment 3

[Karl Tomlinson (:karlt)]

(In reply to comment #2)
> I'm seeing this on 10.4.11 and a german keyboard on an en-US build.

What's your build id?

> That doesn't affect that '?' does not open the help window, though, right?

Maybe the 10.4/German issue has a different cause.

Comment 4

[Axel Hecht [:Pike]]

Mozilla/5.0 (Macintosh; U; PPC Mac OS X 10.4; en-US; rv:1.9pre) Gecko/2008050704 Minefield/3.0pre, I updated before commenting, too :-)

Comment 5

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(sickish, maybe slower respose)]

(In reply to comment #3)
> (In reply to comment #2)
> > That doesn't affect that '?' does not open the help window, though, right?
> 
> Maybe the 10.4/German issue has a different cause.

Ah, I can reproduce it. But I confirm that this is fixed by bug 432388. unshiftedCmdChar is not same as shiftedCmdChar at the key on German layout. That issue is fixed by bug 432388. It's really different bug.

Comment 6

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(sickish, maybe slower respose)]

Ugh,... I have no idea for this bug.

Can we use new APIs with dynamic link only on 10.5? But I don't know how to do on Mac.
http://developer.apple.com/documentation/TextFonts/Reference/TextInputSourcesReference/Reference/reference.html

Comment 7

[Karl Tomlinson (:karlt)]

(In reply to comment #6)
> Can we use new APIs with dynamic link only on 10.5?

AFAIK, but we need someone with 10.5 to do this.
(See bug 306585 comment 38.)

It may also be worth trying deprecated Keyboard Layout Services, which would be available on 10.4 too.

bug 432388 comment 6
http://lists.apple.com/archives/cocoa-dev/2008/Apr/msg01582.html

Comment 8

[Karl Tomlinson (:karlt)]

Attachment: TIS test code

So we seem to get a uchr for Swedish layout with TIS.

dlopening /System/Library/Frameworks/Carbon.framework/Frameworks/HIToolbox.framework/Versions/A
should be possible.

Comment 9

[Josh Aas]

Attachment: fix v1.0

Karl's patch plus dlopen/dlsym.

Masayuki, does this work fine on 10.4?

Comment 10

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(sickish, maybe slower respose)]

Comment on attachment 319919 [details] [diff] [review]
fix v1.0

> +        TISInputSourceRef tis = Leopard_TISCopyCurrentKeyboardLayoutInputSource();
> +        uchr = static_cast<CFDataRef>(Leopard_TISGetInputSourceProperty(tis, kTISPropertyUnicodeKeyLayoutData));        

Don't you need to define TISInputSourceRef and kTISPropertyUnicodeKeyLayoutData for 10.4?

Comment 11

[Josh Aas]

Attachment: fix v1.1

Yeah, sorry.

Comment 12

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(sickish, maybe slower respose)]

Comment on attachment 319922 [details] [diff] [review]
fix v1.1

> +      CFDataRef uchr = NULL;
> +      if (nsToolkit::OnLeopardOrLater() &&
> +          Leopard_TISCopyCurrentKeyboardLayoutInputSource &&
> +          Leopard_TISGetInputSourceProperty) {
> +        TISInputSourceRef tis = Leopard_TISCopyCurrentKeyboardLayoutInputSource();
> +        uchr = static_cast<CFDataRef>(Leopard_TISGetInputSourceProperty(tis, kTISPropertyUnicodeKeyLayoutData));        
> +      }

I think if gOverrideKeyboardLayout is not null, we should not set uchr from current keyboard layout.

Comment 13

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix v1.2

The typedef was flipped. I fixed that, and the patch builds and runs fine. The existing test_keycodes.xul tests pass. Some extra tests I have in my tree also pass.

Comment 14

[Robert O'Callahan (:roc) (email my personal email if necessary)]

The test in comment #0 also works for me with the patch. This is all on 10.4.

Comment 15

[Josh Aas]

Attachment: fix v1.3

Comment 16

[Josh Aas]

Try server build with "fix v1.3".

https://build.mozilla.org/tryserver-builds/2008-05-08_09:[e-mail]-1210265934/[e-mail]

Comment 17

[Josh Aas]

fix v1.3:
Build 10.4, run on 10.5 -> Fixed
Build 10.5, run on 10.5 -> Fixed
Build 10.4, run on 10.4 -> ?

Comment 18

[Robert O'Callahan (:roc) (email my personal email if necessary)]

The build in comment #16 works for me on 10.4

Comment 19

[Stefan [:stefanh]]

(In reply to comment #18)
> The build in comment #16 works for me on 10.4
> 

For me too. Hmm, too bad I'm on 10.4 today when there's a fix for a 10.5-bug that I've reported :-/

Comment 20

[Karl Tomlinson (:karlt)]

Comment on attachment 319937 [details] [diff] [review]
fix v1.3

+        uchr = static_cast<CFDataRef>(Leopard_TISGetInputSourceProperty(tis, (CFStringRef)kOurTISPropertyUnicodeKeyLayoutData));        

we don't need the CFStringRef cast here.

Earlier versions of the patch were tested on 10.4 so let's go.

Comment 21

[Karl Tomlinson (:karlt)]

(In reply to comment #12)
> I think if gOverrideKeyboardLayout is not null, we should not set uchr from
> current keyboard layout.

Yes, tests won't work on 10.5, but they (some) don't pass now anyway and there are advantages and disadvantages with using or not using current layout depending on whether the layout can be found by GetResource (not being found being this bug).  Hopefully we can get tests working on 10.5 through bug 432533.

Comment 22

[Karl Tomlinson (:karlt)]

BTW, this patch may add to shutdown leaks if the library is not already open due to the missing dlclose, but this side-effect of the cure is better than the disease.

Comment 23

[Damon Sicore (:damons)]

Comment on attachment 319937 [details] [diff] [review]
fix v1.3

a1.9+=damons

Comment 24

[Josh Aas]

landed