[Mac]Regression: Cmd+{ and Cmd+} will not switch tabs

RESOLVED FIXED

Status

()

Core
Keyboard: Navigation
RESOLVED FIXED
10 years ago
9 years ago

People

(Reporter: Louise, Unassigned)

Tracking

({regression})

Trunk
All
Mac OS X
regression
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.9 -
in-litmus +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

10 years ago
User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9pre) Gecko/2008041421 Minefield/3.0pre
Build Identifier: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9pre) Gecko/2008041421 Minefield/3.0pre ID:2008041421

⌘+{ and ⌘+} on US keyboards, standard keyboard shortcuts for switching tabs in OS X stopped working between build 20080414_2047 and
20080414_2130, regression of Bug 417176 possibly caused by Bug 359638.




Reproducible: Always

Steps to Reproduce:
1.Open more than one tab in single window.
2.Hit Cmd+{ or Cmd+}
Actual Results:  
Mac gives a beep

Expected Results:  
Firefox should switch between tabs.
(Reporter)

Updated

10 years ago
Blocks: 359638
Keywords: regression
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: blocking1.9?
Version: unspecified → Trunk
We didn't block on bug 417176, so I don't think we'd block on this bug, either, as much as it pains me.

Bug 359638 broke a lot of stuff, though, so I expect that the regression will be addressed (I've put a reference to it on that bug, too)
Flags: blocking1.9? → blocking1.9-
(In reply to comment #0)
> 2.Hit Cmd+{ or Cmd+}

Is the Shift key down?

Comment 3

10 years ago
(In reply to comment #2)
> (In reply to comment #0)
> > 2.Hit Cmd+{ or Cmd+}
> 
> Is the Shift key down?
> 

In my case, yes (Standard Apple JIS aluminium keyboard).

Comment 4

10 years ago
Created attachment 315900 [details] [diff] [review]
handle new charCode values

Tweaking the extension I used in bug 417176, based on the info in bug 359638, led me to this patch.

If the latter is sticking around, could someone r? this patch appropriately?
Bug 359638 is a blocker, so we shouldn't need that patch.
No that should not be able to fix this bug. Because the charCode is '[' or ']', not '{' or '}'. We need to create new <key/>.
Sorry, bug 359638's state confused me. What do you mean by "add a new key"? Do you mean that the behavior change was expected, and that we should take Jacob's patch?
(Reporter)

Comment 8

10 years ago
(In reply to comment #2)
> (In reply to comment #0)
> > 2.Hit Cmd+{ or Cmd+}
> 
> Is the Shift key down?
> 

yes, in MacBook US keyboard(Cmd+Shift+[).  I don't know how the shortcuts are dealed in other languages' keyboard layout.
(In reply to comment #7)
> Sorry, bug 359638's state confused me. What do you mean by "add a new key"?

I think Masayuki is suggesting that this be implemented using a XUL <key>
element because the XBL key event handler has special code to use extra
information in the native key event which is not available to javascript.
I agree that a <key> is the best way to do this but...

(In reply to comment #6)
> No that should not be able to fix this bug. Because the charCode is
> '[' or ']', not '{' or '}'.

The behavior here has changed and I fear this behavior change could affect
other javascript code.  This new behavior is also inconsistent with
other platforms IIUC.

Masayuki, I know we agreed that we should send the character code from the
event, but this particular case (Cmd+Shift) seems to be a deficiency of the
"characters" field of the NSEvent, which is provided for through the
charactersIgnoringModifiers field.

It seems that when Cmd+Shift are down, "characters" is always the same as when
on Cmd is down.  So this field seems to be of little use in the Cmd+Shift
situation.

I would recommend setting charCode from charactersIgnoringModifiers whenever
Cmd+Shift are down and shiftedCmdChar == unshiftedChar.
(In reply to comment #9)
> It seems that when Cmd+Shift are down, "characters" is always the same as when
> on Cmd is down.

I mean _only_ Cmd is down.
The fact that bug 359638 made us see different charCodes for these events scares me a lot from a compatibility point of view - it looks like a number of bugs have been caused by the change, and who knows how much out-of-tree code is depending on the old behavior. Was there no lower-impact way of fixing bug 359638?
> Was there no lower-impact way of fixing bug 359638?

If I know that, I did it.
(In reply to comment #12)
> > Was there no lower-impact way of fixing bug 359638?
> 
> If I know that, I did it.

On Windows and Linux, when Ctrl key is pressed, the may send the ASCII control characters. Then, we can replace the charCode to ASCII normal characters like Cocoa.

widget/src/cocoa/nsChildView:
> 3890     // convert control-modified charCode to raw charCode (with appropriate case)
> 3891     if (outGeckoEvent->isControl && outGeckoEvent->charCode <= 26)
> 3892       outGeckoEvent->charCode += (outGeckoEvent->isShift) ? ('A' - 1) : ('a' - 1);

Otherwise, we should keep the current behavior. But this issue is really another bug.
karlt:

I filed bug 429510. I will replace the charCode at most cases like previous behavior.
Depends on: 429510
Flags: in-litmus?
fixed by bug 432389.

-> FIXED
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
https://litmus.mozilla.org/show_test.cgi?id=7715 to Litmus in both the FFT and the Mac keyboard specific test run.
Flags: in-litmus? → in-litmus+
You need to log in before you can comment on or make changes to this bug.