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

Status: VERIFIED FIXED

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

Description

[[e-mail]]

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 `

Comment 1

[Louise]

WFM in 10.5.2 with latest trunk.

Maybe it was fixed between b3 and trunk. Bug 406970?

Comment 2

[[e-mail]]

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.

Comment 4

[Shawn Wilsher :sdwilsh]

WFM in
Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9b4pre) Gecko/2008021704 Minefield/3.0b4pre

Comment 5

[dw]

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.

Thanks.

Comment 6

[dw]

Please reopen this bug.

Comment 7

[Shawn Wilsher :sdwilsh]

reopening per comment 5.  can we get someone with 10.5 to check this out please?

Comment 8

[dw]

I just tried the build that you used when marking the ticket WFM - nope, it's definitely in that build too.

Thanks,


David

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

Comment 9

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

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 http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=PhoenixTinderbox&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2008-01-28+04%3A00&maxdate=2008-01-29+04%3A00&cvsroot=%2Fcvsroot
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....

Comment 10

[[e-mail]]

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.

Comment 11

[[e-mail]]

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:
http://developer.apple.com/documentation/mac/Toolbox/graphics/Figure_1-10_EV-17.jpg

Comment 12

[[e-mail]]

Sorry, I meant the patch in bug 376077 not 407793.

Comment 13

[[e-mail]]

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.

Comment 14

[[e-mail]]

Attachment: workaround

Comment 15

[Henry Walker]

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.

Comment 16

[Josh Aas]

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.

Comment 17

[Josh Aas]

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.

Comment 18

[Josh Aas]

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.

Comment 19

[Josh Aas]

Attachment: fix v1.0

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.

Comment 20

[Josh Aas]

Attachment: fix v1.1

Updated to current trunk

Comment 21

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

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...

Comment 22

[Josh Aas]

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.

Comment 23

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

+        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?

Comment 24

[Josh Aas]

Attachment: fix v1.2

Comment 25

[Josh Aas]

landed on trunk

Comment 26

[Marcia Knous [:marcia]]

verified fixed using Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9b5pre) Gecko/2008031704 Minefield/3.0b5pre.

Comment 27

[Godefroid Chapelle]

This bug has not been totally fixed.

see

https://bugzilla.mozilla.org/show_bug.cgi?id=440961

Comment 28

[Joshua Mitchell (Inactive)]

Issue is Resolved - removing QA-Wanted Keywords - QA-Wanted query clean-up task