1335218 - duplicate "l" access key for Highlight All and Replace

Status: RESOLVED FIXED

(Thunderbird :: Message Compose Window, defect)

Description

[Wayne Mery (:wsmwk)]

52.0b2 and nightly have duplicate "l" access key for Highlight All and Replace. Annoying. 51.0b2 does not have this problem.

Compose message
put cursor in message body
ctrl+f
see "l" in both places.

Comment 1

[Jorg K (CEST = GMT+2)]

Richard, could you take a look, please. Where did this access key come from in TB 52 and later?

Comment 2

[Richard Marti (:Paenglab)]

Bug 435326 changed on 8. November 2016 the accesskes from "a" to "l" which we already used for our replace button. The question is, what key should we use?

Comment 3

[Wayne Mery (:wsmwk)]

wow. There's nothing else available in the word replace. Uness you change menu Options to alt+O and free the "p" for replace

Comment 4

[Richard Marti (:Paenglab)]

alt+O is already used for the Format menu.

Comment 5

[Jorg K (CEST = GMT+2)]

I don't get it:
Before:
Highlight All was using a.
Replace was using l.

Now:
Highlight All is using l.
Replace is using l.

So clearly I'd use a for Replace. Where is the problem?

Comment 6

[Richard Marti (:Paenglab)]

Because from bug 435326 comment 0:
> Mac OS X uses emacs keybindings for all dialog boxes.
> 
> some very common keybindings are 
> control-p previous line
> control-n next line
> control-a beginning of line
> control-e end of line
> 
> however when the findbar is open (command-F) the control-a key
> is used to toggle "Highlight all" function on and off.

On macOS instead of alt ctrl is used.

I tried all keys (with address sidebar open) and these are free, also on macOS: g j m u x y z
I propose "x" as this could mean eXchange instead of replace. What do you think?

Comment 7

[Jorg K (CEST = GMT+2)]

I'd remove the shortcut key "l" for Replace. Otherwise "x" is the best choice. How does that look?
Replace...(x)?

Comment 8

[Richard Marti (:Paenglab)]

Attachment: Bug1335218.patch

Changed the accesskey to "x".

Also moved the Replace button after "Whole Words" to be the last again (and the separator makes sense again).

Comment 9

[Jorg K (CEST = GMT+2)]

Comment on attachment 8832630 [details] [diff] [review]
Bug1335218.patch

That patch doesn't apply since you've got the patch from bug 394216 applied ;-)
Big brother is watching you.

Comment 10

[Jorg K (CEST = GMT+2)]

Attachment: Bug1335218.patch

This one is better ;-)

Comment 11

[Jorg K (CEST = GMT+2)]

Comment on attachment 8832635 [details] [diff] [review]
Bug1335218.patch

Please fix the commit message before landing this:
Bug 1335218 - Use other accesskey for thr "Replace" button because the old is used by "Highlight all". r=jorgk

"the".

Comment 12

[Richard Marti (:Paenglab)]

Attachment: Bug1335218.patch

Patch without big brother enabled content.

Comment 13

[Jorg K (CEST = GMT+2)]

Richard, I've already fixed your patch, see comment #10 and comment #11, there was no need to do it again ;-). Also, when landing, please fix the commit message, see comment #11.

Comment 14

[Richard Marti (:Paenglab)]

https://hg.mozilla.org/comm-central/rev/2945ffabba5d22ed6382495d4cec193000263c0b

Comment 15

[Richard Marti (:Paenglab)]

Comment on attachment 8832642 [details] [diff] [review]
Bug1335218.patch

[Approval Request Comment]
Regression caused by (bug #): 435326
User impact if declined: not correctly working accesskeys
Testing completed (on c-c, etc.): on c-c
Risk to taking this patch (and alternatives if risky): low

Comment 16

[Jorg K (CEST = GMT+2)]

Aurora (TB 53):
https://hg.mozilla.org/releases/comm-aurora/rev/1554c43e520d55b8da9ecb0e2619eefc0da9f318

Comment 17

[Jorg K (CEST = GMT+2)]

Beta (TB 52):
https://hg.mozilla.org/releases/comm-beta/rev/ab6902662b94651e1603423714986b07210a974e