Mac: Redo command keyboard shortcut should be shown as shift + Undo keyboard shortcut, since that's what the backend implements

VERIFIED FIXED in Thunderbird 3.1b1

Status

Thunderbird
OS Integration
VERIFIED FIXED
8 years ago
7 years ago

People

(Reporter: rolandtanglao, Assigned: philor)

Tracking

({regression})

Trunk
Thunderbird 3.1b1
All
Mac OS X
regression
Dependency tree / graph

Thunderbird Tracking Flags

(thunderbird3.0 .2-fixed)

Details

Attachments

(1 attachment, 2 obsolete attachments)

On Mac under TB 3.0 RC1 cmd-Y (Redo) doesn't work while editing an email. Only by going through the menu using Edit > Redo will the command work. Command Z i.e. undo works.

The equivalent Windows command works i.e. Control Z and Control Y both work for undo and re-do.
I took a look at this the other day and couldn't see anything obvious. I'm kinda hoping Phil might have some ideas.

Certainly would be nice to get a fix for this into a dot release if we can do something without affecting strings.
status-thunderbird3.0: --- → wanted
Flags: blocking-thunderbird3.1?
(Assignee)

Comment 3

8 years ago
The only obvious thing I see so far is that we use different shortcuts in compose and the 3pane, and they are both wrong: neither cmd+Y nor cmd+shift+Y is the cmd+shift+Z that should be redo on OS X.
Keywords: regression
(In reply to comment #3)
> The only obvious thing I see so far is that we use different shortcuts in
> compose and the 3pane, and they are both wrong: neither cmd+Y nor cmd+shift+Y
> is the cmd+shift+Z that should be redo on OS X.

Yeah I agree with the cmd-shift-z, just hoping for a possible branch fix as well.
(Assignee)

Comment 5

8 years ago
Weird news and good news:

The weird news is that using accel+shift+undoCmd.key for Redo (the cmd+shift+z that it should be) actually fixes this bug. That seems like it must be a widget: cocoa bug, since it shouldn't be paying any attention at all to whether that commandkey is shifted, or is a particular letter, or is the same letter as another command.

The good news is that there are very very few locales that use anything other than Z for undoCmd.key and Y for redoCmd.key, and all the ones that do look like bugs, rather than intentionally different because that locale typically uses something other than accel+Z and either accel+Y or accel+shift+Z, so I think despite it seeming like a l10n-impacting change that we couldn't consider on a branch, it really isn't.

sr uses different letters for both only in the addressbook, where one looks like it was mistakenly chosen like a (duplicated) accesskey rather than a commandkey and the other doesn't have any clear reason at all unless Д is the capitalized version of one of the lowercase letters in Понови (making it another mistaken accesskey-style choice), and pt-PT uses a different letter for undoCmd.key only, only in compose, also looking like it was mistakenly chosen like an accesskey rather than a commandkey.

We can also take some comfort in the fact that if there was a localization-related issue (which would come up if there was a locale where Linux/Mac typically use "A" and "B" rather than "A" and "shift+A"), it would have long since shaken out in browser/, where bug 258788 made this same change in 2004.
(Assignee)

Updated

8 years ago
Depends on: 532310
(Assignee)

Updated

8 years ago
Depends on: 532311
(Assignee)

Comment 6

8 years ago
Oh, bless your twisted sociopathic heart, widget: cocoa.

cmd+shift+Z already works for redo - apparently redoCmd.key is just for show, and no matter what the menu claims is the commandkey, the thing that always and only works is cmd+shift+Z.

And that raises another "fun" branch driving question: do we switch both Mac and Linux over to what they should be on the trunk, and then only switch Mac over to displaying the thing that works, leaving Linux with its current working but wrong commandkey, or do we switch them both?
(Assignee)

Comment 7

8 years ago
Created attachment 415563 [details] [diff] [review]
Trunk fix

And since "whatever will frustrate me the most" is always the correct answer for branch driving, I think the answer is "yes, that's just what we do."

Trunk version, fixing both Mac and Linux to both use and show accel+shift+Z.
Attachment #415563 - Flags: review?(bugzilla)
(Assignee)

Comment 8

8 years ago
Created attachment 415566 [details] [diff] [review]
Actual trunk fix

I really need to develop a better patch naming scheme, that would keep me from attaching redo-key-dude-this-is-just-the-testing-patch-for-just-the-compose-window-not-all-three.
Assignee: nobody → philringnalda
Attachment #415563 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #415566 - Flags: review?(bugzilla)
Attachment #415563 - Flags: review?(bugzilla)
(Assignee)

Comment 9

8 years ago
Created attachment 415569 [details] [diff] [review]
Branch fix

Just switching Mac over to displaying the shortcut that actually works, without the Linux switch to what it should be.
Attachment #415569 - Flags: review?(bugzilla)
(Assignee)

Updated

8 years ago
Flags: in-testsuite?
Comment on attachment 415569 [details] [diff] [review]
Branch fix

Yep, this works and considering your comments I think this is the right things to do for branch.

Can you land this on trunk for a few days though? Then we'll move it across to branch soon?
Attachment #415569 - Flags: review?(bugzilla) → review+
(Assignee)

Updated

8 years ago
Attachment #415566 - Attachment is obsolete: true
Attachment #415566 - Flags: review?(bugzilla)
(Assignee)

Comment 11

8 years ago
Comment on attachment 415566 [details] [diff] [review]
Actual trunk fix

Yep, but doing so will force me to accept that this is actually two separate bugs, a trunk+3.0 "Mac: show shift+undoCmd.key as the shortcut for Redo, since that's what the backend implements" and a trunk "Linux: Redo shortcut should be shift+undoCmd.key"
(Assignee)

Updated

8 years ago
Hardware: x86 → All
Summary: Redo Command keyboard shortcut doesn't work in Mac TB 3.0 RC1 → Mac: Redo command keyboard shortcut should be shown as shift + Undo keyboard shortcut, since that's what the backend implements
Version: 3.0 → Trunk
(Assignee)

Updated

8 years ago
Blocks: 540476
(Assignee)

Comment 12

8 years ago
http://hg.mozilla.org/comm-central/rev/05b230cb0623
No longer blocks: 540476
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 3.2a1
Flags: blocking-thunderbird3.1?
Target Milestone: Thunderbird 3.2a1 → Thunderbird 3.1b1
(Assignee)

Updated

8 years ago
Attachment #415569 - Flags: approval-thunderbird3.0.2?
Attachment #415569 - Flags: approval-thunderbird3.0.2? → approval-thunderbird3.0.2+
(Assignee)

Comment 13

8 years ago
http://hg.mozilla.org/releases/comm-1.9.1/rev/ce315beaf3b8
status-thunderbird3.0: wanted → .2-fixed
Status: RESOLVED → VERIFIED
Keywords: verified-thunderbird3.0
(Assignee)

Updated

7 years ago
Flags: in-testsuite?
You need to log in before you can comment on or make changes to this bug.