duplicate "l" access key for Highlight All and Replace

RESOLVED FIXED in Thunderbird 54.0

Status

defect
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: wsmwk, Assigned: Paenglab)

Tracking

({regression})

52 Branch
Thunderbird 54.0
Dependency tree / graph

Thunderbird Tracking Flags

(thunderbird52 fixed, thunderbird53 fixed, thunderbird54 fixed)

Details

(Whiteboard: [regression:TB52])

Attachments

(1 attachment, 2 obsolete attachments)

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.
Blocks: TB52found
Richard, could you take a look, please. Where did this access key come from in TB 52 and later?
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?
wow. There's nothing else available in the word replace. Uness you change menu Options to alt+O and free the "p" for replace
Blocks: 435326
alt+O is already used for the Format menu.
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?
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?
I'd remove the shortcut key "l" for Replace. Otherwise "x" is the best choice. How does that look?
Replace...(x)?
Posted patch Bug1335218.patch (obsolete) — Splinter Review
Changed the accesskey to "x".

Also moved the Replace button after "Whole Words" to be the last again (and the separator makes sense again).
Assignee: nobody → richard.marti
Status: NEW → ASSIGNED
Attachment #8832630 - Flags: review?(jorgk)
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.
Attachment #8832630 - Flags: review?(jorgk)
Posted patch Bug1335218.patch (obsolete) — Splinter Review
This one is better ;-)
Attachment #8832630 - Attachment is obsolete: true
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".
Attachment #8832635 - Flags: review+
Patch without big brother enabled content.
Attachment #8832635 - Attachment is obsolete: true
Attachment #8832642 - Flags: review?(jorgk)
Attachment #8832635 - Attachment is obsolete: false
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.
Attachment #8832642 - Flags: review?(jorgk) → review+
https://hg.mozilla.org/comm-central/rev/2945ffabba5d22ed6382495d4cec193000263c0b
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 54.0
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
Attachment #8832642 - Flags: approval-comm-beta?
Attachment #8832642 - Flags: approval-comm-aurora?
Attachment #8832635 - Attachment is obsolete: true
Attachment #8832642 - Flags: approval-comm-beta?
Attachment #8832642 - Flags: approval-comm-beta+
Attachment #8832642 - Flags: approval-comm-aurora?
Attachment #8832642 - Flags: approval-comm-aurora+
You need to log in before you can comment on or make changes to this bug.