Format menu accelerators display "Shift" then "Ctrl"

VERIFIED FIXED in mozilla0.9.2

Status

()

Core
Editor
P4
trivial
VERIFIED FIXED
17 years ago
16 years ago

People

(Reporter: Ninoschka Baca, Assigned: Charles Manske)

Tracking

Trunk
mozilla0.9.2
x86
Windows ME
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

17 years ago
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)
(Reporter)

Updated

17 years ago
Keywords: nsCatFood

Comment 1

17 years ago
Created attachment 32789 [details] [diff] [review]
patch

Comment 2

17 years ago
r=Hurricane

Comment 3

17 years ago
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

17 years ago
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

17 years ago
assigning to cmanske
Assignee: beppe → cmanske
Severity: normal → trivial
Keywords: correctness
Priority: -- → P4
Target Milestone: --- → mozilla0.9.3
(Assignee)

Comment 6

17 years ago
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.
Status: NEW → ASSIGNED
Whiteboard: FIX IN HAND need SR=
Target Milestone: mozilla0.9.3 → mozilla0.9.2

Comment 7

17 years ago
sr=kin@netscape.com

Did someone already file that RFE bug?

Comment 8

17 years ago
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.
(Assignee)

Comment 9

17 years ago
Removing "need sr=" from status
Whiteboard: FIX IN HAND need SR= → FIX IN HAND
(Assignee)

Comment 10

17 years ago
checked in.
Status: ASSIGNED → RESOLVED
Last Resolved: 17 years ago
Keywords: correctness, nsCatFood
Resolution: --- → FIXED
Whiteboard: FIX IN HAND

Comment 11

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

Comment 12

17 years ago
I filed the general RFE bug.

Comment 13

17 years ago
The general bug is #79491

Comment 14

17 years ago
verified.....
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.