Closed Bug 417466 Opened 16 years ago Closed 16 years ago

Switching windows with Command backtick (cmd `) not working.


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






(Reporter: me, Assigned: jaas)



(Keywords: regression)


(1 file, 3 obsolete files)

User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9b3) Gecko/2008020511 Firefox/3.0b3
Build Identifier: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9b3) Gecko/2008020511 Firefox/3.0b3

(Okay, I really searched for this bug.  It seems really obvious, yet I couldn't find it.  If this is a duplicate, sorry for the noise.)

On a fresh install of 3.0b3 with a fresh profile, I can't switch between windows using Cmd-`.

Reproducible: Always

Steps to Reproduce:
1. Command N
2. Command `
WFM in 10.5.2 with latest trunk.

Maybe it was fixed between b3 and trunk. Bug 406970?
No, still broken in trunk. However, I did some research:

It works in the 2008-01-28-04 nightly.  It's broken in the 2008-01-29-04 nightly.
WFM in
Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9b4pre) Gecko/2008021704 Minefield/3.0b4pre
Closed: 16 years ago
Resolution: --- → WORKSFORME
Shawn - this appears to be a bug affecting Leopard - ie. 10.5.x, not 10.4.

I can confirm this doesn't work in trunk as of 2008021804. To repeat, I'm on Leopard 10.5.2. I'm using a UK keyboard with US layout set. No matter where I click on either Download Manager or Firefox windows, Cmd-` doesn't function. Nor does my shortcut for the Zoom menu item as set in the keyboard prefpane.

Please reopen this bug.
reopening per comment 5.  can we get someone with 10.5 to check this out please?
Keywords: qawanted
Resolution: WORKSFORME → ---
Assignee: nobody → joshmoz
Component: OS Integration → Widget: Cocoa
Product: Firefox → Core
QA Contact: os.integration → cocoa
Version: unspecified → Trunk
I just tried the build that you used when marking the ticket WFM - nope, it's definitely in that build too.



Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9b4pre) Gecko/2008021704 Minefield/3.0b4pre
I can't reproduce this on 10.5.2 with 

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9b4pre) Gecko/2008021804 Minefield/3.0b4pre

The regression range mentioned in comment 2 is
which could possibly mean bug 376077 caused it.

Everyone who is seeing this is using a non-US keyboard or non-US keyboard layout, right?  That reminds me of bug 407793....
I think you're on to something. I'm using a German keyboard, but with US or Dvorak layout.  (Neither works)

I also just found out that changing the global shortcut to some other key (say cmd-f7) makes it work again, so really only the backtick key is affected.  And by that I mean the key below ESC.  If backtick is mapped somewhere else (e.g. Greek layout), it also works.

Note that German keyboards have a different number of keys and I believe a different keycode for said key.
Just looked at the patch in bug 407793.  It adds a hardcoded keycode 0x32 for the tilde key.  As I said, I think it's different for international keyboards, as this image suggests:
Sorry, I meant the patch in bug 376077 not 407793.
To demonstrate the issue I've written a work-around that checks for both possible tilde keycodes.  It's not the right thing to do.  It should instead check what key is configured for window-switching.  But the current code has that wrong, too.
Attached patch workaround (obsolete) — Splinter Review
Blocks: 376077
I have an associated problem to this one - I use the UK keyboard layout in OS X 10.5.2 and have changed the Keyboard Shortcut for 'Move focus to next window in active application' from Cmd+` to Cmd+§ (as it's easier to reach on the UK layout).

In Firefox 3.0b3, with the OS shortcut set to Cmd+§, switching windows doesn't work.  Setting the shortcut back to Cmd+` makes it work again.  From the looks of the attachment in Comment #14 the keycode for window switching is hard-coded and not inherited from the OS.

This is a regression from Firefox 2 where Cmd+§ works to switch windows.
Flags: blocking1.9?
Keywords: regression
Flags: blocking1.9? → blocking1.9+
Priority: -- → P1
Ever confirmed: true
Gecko views are a black hole for key events since we don't know if they get handled reliably when we send them into Gecko. The problem is that if the window-switching key is not the one for normal US keyboard layouts, we eat the event and the OS doesn't run the default handler.

We have a couple options here. We could assume that gecko cmd+something handlers return a reasonable handled status and use that to decide whether or not to eat the event. The up side to this approach is that everyone's bug here gets fixed. The downside is that if the gecko handled status is inaccurate according to our expectations then the key command might cause an action to be taken in gecko, then the OS will run the default handler and also either beep or do something else.

Another option is to code for default layouts. This means special-casing a set of default keycodes or characters, which maps to the default window-switching keys for intl layouts. The plus side here is we avoid the bad case above, the minus is that people who don't use the default keys are left out by the fix. That means Henry in comment #15.

I'll have to think about this a bit more.
A third option is to figure out how to figure out what the window switching key is at runtime. This would work and have no down side. I doubt that is going to happen though.
For the record, bug 376077 is the primary bug about performing an action successfully but letting the default OS handler run so it also beeps. Whatever we do we shouldn't regress that.
Attached patch fix v1.0 (obsolete) — Splinter Review
I think the right thing to do here is to trust Gecko's handled status for key presses that have the command modifier flag set. For all other key events we still don't trust Gecko's handled status. Most of the inaccurate handled statuses do not happen when the cmd-key is down, and the ones that do we can probably fix easily. Any OS functionality like window switching will most likely use the cmd key modifier. This is really the best option we have, it actually makes a lot of sense.

This patch will regress bug 376077, so we'll have to re-open that and fix it as Jesse suggested in bug 376077 comment #5.
Attachment #305724 - Attachment is obsolete: true
Attachment #308554 - Flags: review?
Attachment #308554 - Flags: review? → review?(masayuki)
Attached patch fix v1.1 (obsolete) — Splinter Review
Updated to current trunk
Attachment #308554 - Attachment is obsolete: true
Attachment #308968 - Flags: review?
Attachment #308554 - Flags: review?(masayuki)
Attachment #308968 - Flags: review? → review?(masayuki)
Comment on attachment 308968 [details] [diff] [review]
fix v1.1

> -    return;
> +    return (mKeyDownHandled || mKeyPressHandled);;

The semicolon is doubled.

Otherwise looks ok now... However, it seems that we need to fix the key event black hole issue after Gecko1.9...
Attachment #308968 - Flags: superreview?(roc)
Attachment #308968 - Flags: review?(masayuki)
Attachment #308968 - Flags: review+
There is no way around not trusting the event return value and I don't think it is planned to change that even after 1.9. I think this just hits us harder because the Mac typically provides more feedback to users when they hit a keyboard command. We have no choice but to trust in the command-key case though, which is good because the command return values are most reliable.
+        PRBool rv = mKeyDownHandled;

Don't use rv for non-nsresults, it's confusing. Call it 'handled' or something.
(3 occurrences)

+    unsigned int modifierFlags = [theEvent modifierFlags] & NSDeviceIndependentModifierFlagsMask;

Why are you regetting the flags here?
Attached patch fix v1.2Splinter Review
Attachment #308968 - Attachment is obsolete: true
Attachment #309275 - Flags: superreview?(roc)
Attachment #308968 - Flags: superreview?(roc)
Attachment #309275 - Flags: superreview?(roc) → superreview+
landed on trunk
Closed: 16 years ago16 years ago
Resolution: --- → FIXED
verified fixed using Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9b5pre) Gecko/2008031704 Minefield/3.0b5pre.
This bug has not been totally fixed.

Issue is Resolved - removing QA-Wanted Keywords - QA-Wanted query clean-up task
Keywords: qawanted
You need to log in before you can comment on or make changes to this bug.