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

Status: VERIFIED FIXED

(Thunderbird :: OS Integration, defect)

Description

[Roland Tanglao needinfo please :rolandtanglao, :mohnkuchen, :adobo, :sinigang, :roland]

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.

Comment 1

[Roland Tanglao needinfo please :rolandtanglao, :mohnkuchen, :adobo, :sinigang, :roland]

reported in get satisfaction at:
http://getsatisfaction.com/mozilla_messaging/topics/redo_command_bug_in_mac_tb_3_0_rc1?utm_content=topic_link&utm_medium=email&utm_source=new_topic

Comment 2

[Mark Banner (:standard8)]

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.

Comment 3

[Phil Ringnalda (:philor)]

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.

Comment 4

[Mark Banner (:standard8)]

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

Comment 5

[Phil Ringnalda (:philor)]

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.

Comment 6

[Phil Ringnalda (:philor)]

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?

Comment 7

[Phil Ringnalda (:philor)]

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

Comment 8

[Phil Ringnalda (:philor)]

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

Comment 9

[Phil Ringnalda (:philor)]

Attachment: Branch fix

Just switching Mac over to displaying the shortcut that actually works, without the Linux switch to what it should be.

Comment 10

[Mark Banner (:standard8)]

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?

Comment 11

[Phil Ringnalda (:philor)]

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"

Comment 12

[Phil Ringnalda (:philor)]

http://hg.mozilla.org/comm-central/rev/05b230cb0623

Comment 13

[Phil Ringnalda (:philor)]

http://hg.mozilla.org/releases/comm-1.9.1/rev/ce315beaf3b8