Closed Bug 432632 Opened 13 years ago Closed 13 years ago

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

Categories

(Core :: Widget: Cocoa, defect)

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: stefanh, Assigned: jaas)

References

Details

(Whiteboard: [key hell])

Attachments

(1 file, 4 obsolete files)

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?
See bug 306585 comment 15 and bug 306585 comment 36.

This may be a serious bug of MacOS X 10.5.
Flags: blocking1.9?
Summary: Cmd+? (in shifted position) doesn't work on certain keyboard layouts → [10.5] Cmd+? (in shifted position) doesn't work on certain keyboard layouts
Whiteboard: [key hell]
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?
(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.
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 :-)
(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.
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
(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
Flags: blocking1.9? → blocking1.9+
Attached patch TIS test code (obsolete) — Splinter Review
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.
Attached patch fix v1.0 (obsolete) — Splinter Review
Karl's patch plus dlopen/dlsym.

Masayuki, does this work fine on 10.4?
Attachment #319897 - Attachment is obsolete: true
Attachment #319919 - Flags: review?(mozbugz)
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?
Attached patch fix v1.1 (obsolete) — Splinter Review
Yeah, sorry.
Attachment #319919 - Attachment is obsolete: true
Attachment #319922 - Flags: review?(masayuki)
Attachment #319919 - Flags: review?(mozbugz)
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.
Attached patch fix v1.2 (obsolete) — Splinter Review
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.
Attachment #319922 - Attachment is obsolete: true
Attachment #319922 - Flags: review?(masayuki)
The test in comment #0 also works for me with the patch. This is all on 10.4.
Attachment #319930 - Flags: review+
Attachment #319930 - Flags: review?(mozbugz)
Attachment #319930 - Flags: review?(mozbugz)
Attachment #319930 - Flags: review+
Attached patch fix v1.3Splinter Review
Attachment #319930 - Attachment is obsolete: true
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 -> ?

Attachment #319937 - Flags: review?(mozbugz)
(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 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.
Attachment #319937 - Flags: review?(mozbugz) → review+
Attachment #319937 - Flags: superreview?(roc)
(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.
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.
Attachment #319937 - Flags: superreview?(roc) → superreview+
Attachment #319937 - Flags: approval1.9?
Comment on attachment 319937 [details] [diff] [review]
fix v1.3

a1.9+=damons
Attachment #319937 - Flags: approval1.9? → approval1.9+
landed
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.