78406 - Format menu accelerators display "Shift" then "Ctrl"

Status: VERIFIED FIXED

(Core :: DOM: Editor, defect, P4)

Description

[Ninoschka Baca]

Build 2001-04-30-04: NT4
Build 2001-05-01-04: Mac 9.04

Overview: If an accelerator consists of "Ctrl" and "Shift" it is usually
displayed in the form of "Ctrl+Shift+ and then some letter" (i.e. Ctrl+Shift+T). 

Actual: The Format menu has many items which are displaying with "Shift" first
then "Ctrl": 

- Size|Smaller, Shift+Ctrl+[
- Size|Larger, Shift+Ctrl+]
- Discontinue Text Style, Shift+Ctrl+K
- Discontinue Link, Shift+Ctrl+L

Expected Results: To be consistent with other menu items, I would expect to see
"Ctrl", then "Shift" followed by a letter (i.e. Ctrl+Shift+K instead of
Shift+Ctrl+K)

Comment 1

[Stephen Walker]

Attachment: patch

Comment 2

[Ryan Cassin]

r=Hurricane

Comment 3

[Simon Fraser [no longer active]]

Shouldn't the XUL menu-building code re-order them when building the menu text, 
since this ordering might be different between platforms.

Comment 4

[:deanis74]

Hmmm... that's what I was thinking but now I realize that it's not completely 
straight-forward.  See
  
http://lxr.mozilla.org/seamonkey/source/layout/xul/base/src/nsMenuFrame.cpp#1321
It just builds the string using the modifiers in the order that they're defined.

Maybe we can fix this bug in the xul for now, and file an RFE for 
BuildAcceleratorText() doing the sorting, perhaps using a customized qsort().

Comment 5

[rubydoo123]

assigning to cmanske

Comment 6

[Charles Manske]

Looks good to me, though I agree the order should be determined in
BuildAcceleratorText(). Moving to 0.9.2 since I will checkin the attached fix.

Comment 7

[kinmoz]

sr=[e-mail]

Did someone already file that RFE bug?

Comment 8

[:deanis74]

Kin, I don't think so.  Assign it to me if you want to.  Are there different
orders for different platforms, or is it always the same?  I think it's pretty
much Ctrl+Alt+Shift.  But we can save that discussion for the new bug.

Comment 9

[Charles Manske]

Removing "need sr=" from status

Comment 10

[Charles Manske]

checked in.

Comment 11

[Kathleen :Brade]

I wondered why I had conflicts there!
This is going to continue to regress.  Can someone please file the general bug?

Comment 12

[sujay]

I filed the general RFE bug.

Comment 13

[Kathleen :Brade]

The general bug is #79491

Comment 14

[sujay]

verified.....