Implement Ctrl+H keyboard shortcut for "Find and Replace" on Windows (as in other Windows applications; not on MAC)

RESOLVED FIXED in Thunderbird 24.0

Status

Thunderbird
Message Compose Window
--
enhancement
RESOLVED FIXED
8 years ago
3 years ago

People

(Reporter: Thomas D. (currently busy elsewhere; needinfo?me), Assigned: sshagarwal)

Tracking

(Blocks: 1 bug, {ux-consistency, ux-efficiency})

Trunk
Thunderbird 24.0
All
Windows 8
ux-consistency, ux-efficiency
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

STR

1 in compose window, press Ctrl+H (which will do "Find and Replace" in almost any other text processing application)

Actual

- nothing

Expected

- Show find and replace dialogue
So on Mac at least, this is Cmd-F because we have an integrated dialog that does find & replace.

Cmd-H actually hides the application, as that is standard mac functionality.
Blocks: 530629
Hi Blake,                         ui-review?bwinton

here's a one-minute UI-review:

On Windows, Ctrl+H is the default shortcut for "Find and Replace" on any application that I know, especially Word Processors / Text Editors - even Notepad has it. It's a nice mnemonic because the H is like when you strike out words in a text to replace them with something else.

Unfortunately, Ctrl+H shortcut is not assigned in Thunderbird's composition editor, so windows users (like me) unnecessarily get stuck when they rely on their muscle memory and hit Ctrl+H to "Find and Replace" words in composition (ux-effeciency, ux-consistency with OS). 

Thunderbird's composition has a "Find and Replace" dialogue, so it would be nice to add the default Ctrl+H shortcut for that on Windows and Linux (while keeping the the existing shortcut for "Find" which is Ctrl+F).

Currently, Ctrl+H is not used by Thunderbird on Windows for anything in or outside composition window. On Linux, it's usually the same.

(On MAC, Ctrl+H is reserved for "Minimize Window", so we won't touch that there.
I'll update keyboard documentation accordingly when this lands.)

List of TB keyboard shortcuts (make sure to select the right OS):

https://support.mozillamessaging.com/en-US/kb/keyboard-shortcuts#os=win&browser=tb17

https://support.mozillamessaging.com/en-US/kb/keyboard-shortcuts#w_writing-messages
Flags: needinfo?(bwinton)
OS: All → Windows 8
Summary: Implement Ctrl+H keyboard shortcut for "Find and Replace" (as in other applications) → Implement Ctrl+H keyboard shortcut for "Find and Replace" on Windows (as in other Windows applications; not on MAC)
Version: 3.0 → Trunk
Sounds good to me for Windows.  For Linux I'ld like you to run it by Andreas first.

Thanks,
Blake.
Flags: needinfo?(bwinton) → needinfo?(bugs)
I imagine this little nit needs about 5-10 lines of code to just add the keyboard shortcut to the existing command in composition...

has ui-review+ by Blake in comment 3
-----

This is the command to which the keyboard shortcut should be added:

http://mxr.mozilla.org/comm-central/source/editor/ui/composer/content/ComposerCommands.js#87

87   commandTable.registerCommand("cmd_find",       nsFindCommand);

http://mxr.mozilla.org/comm-central/source/editor/ui/composer/content/ComposerCommands.js#2245

2245 var nsFindCommand =
2246 {
2247   isCommandEnabled: function(aCommand, editorElement)
[snip]
2258       window.openDialog("chrome://editor/content/EdReplace.xul", "_blank",
2259                         "chrome,modal,titlebar", editorElement);
2260     }
Keywords: ux-consistency, ux-efficiency
Whiteboard: [good first bug][has ui-review+]

Comment 5

4 years ago
So you actually propose to open the same Find&Replace dialog when pressing any of Ctrl+F or Ctrl+H ?
(In reply to :aceman from comment #5)
> So you actually propose to open the same Find&Replace dialog when pressing
> any of Ctrl+F or Ctrl+H ?

Well, just for an interim period until we have Bug 530629...

It's Ctrl+F for "Find"
and Ctrl+H for "Replace"

If we don't have separate dialogues for that, than yes, both shortcuts will happen to open the same dialogue. (Fwiw, in M$ Word they also trigger the same dialogue, but on a different active tab for each).

Ideally, do bug 530629 to hook the existing "Find in text"-bar (from *message reader* when you press Ctrl+F) into composition to get rid of that bulky modal "find & replace" dialogue which is always in the way when you just need "find" without replace.

Then we could have Ctrl+F for the find-in-text bar (just find, much better view without the dialogue), and Ctrl+H for the dialogue... (until perhaps somebody clever adds that little "Replace" input box to the find-in-text bar, too...)
(Assignee)

Comment 7

4 years ago
Created attachment 765065 [details] [diff] [review]
Patch

The patch makes both, Ctrl+F and Ctrl+H as the accesskeys for Find and replace. Is this what this asked for, in this bug?
As, I think making Ctrl+F for Find is a part of a separate bug.
Assignee: nobody → syshagarwal
Status: NEW → ASSIGNED
Attachment #765065 - Flags: feedback?(acelists)

Comment 8

4 years ago
Comment on attachment 765065 [details] [diff] [review]
Patch

Review of attachment 765065 [details] [diff] [review]:
-----------------------------------------------------------------

The new Ctrl+H combo is nowhere to be seen in the menu. It just works if the user knows it. Thomas, is that what you wanted?

::: mail/components/compose/content/messengercompose.xul
@@ +222,5 @@
>    <key id="key_selectAll" key="&selectAllCmd.key;" modifiers="accel"/>
>    <key id="key_find" key="&findCmd.key;" command="cmd_find" modifiers="accel"/>
> +#ifndef XP_MACOSX
> +  <key id="key_findReplace" key="&findrCmd.key;" command="cmd_find" modifiers="accel"/>
> +#endif

Let's ask Andreasn if this is wanted on Linux.

::: mail/locales/en-US/chrome/messenger/messengercompose/messengercompose.dtd
@@ +84,5 @@
>  <!ENTITY selectAllCmd.label "Select All">
>  <!ENTITY selectAllCmd.key "A">
>  <!ENTITY selectAllCmd.accesskey "a">
>  <!ENTITY findCmd.label "Find and Replace…">
> +<!ENTITY findrCmd.key "H">

Why can't this be findReplaceCmd.key ?

@@ +89,2 @@
>  <!ENTITY findCmd.key "F">
> +<!ENTITY findrCmd.accesskey "H">

You use this nowhere, it is not needed.
Attachment #765065 - Flags: ui-review?(bugs)
Attachment #765065 - Flags: review?(mkmelin+mozilla)
Attachment #765065 - Flags: feedback?(acelists)
Attachment #765065 - Flags: feedback+
(In reply to :aceman from comment #8)
> Comment on attachment 765065 [details] [diff] [review]
> Patch
> Review of attachment 765065 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> The new Ctrl+H combo is nowhere to be seen in the menu. It just works if the
> user knows it. Thomas, is that what you wanted?

Well, I suppose we can't show two keyboard shortcuts on a single menuitem (unless in a tooltip perhaps?).
It's OK as a hidden shortcut because it's a muscle memory thing, keyboard users will try Ctrl+H automatically even if it's not there in the UI.
It's a first step towards separating Find vs. Find & Replace, we can advertise both shortcuts after implementing bug 530629 to use Ctrl+F -> "Find in text" bar for "Find", vs. Ctrl+H -> Find & Replace dialogue.

Comment 10

4 years ago
We can't show two shortcuts on single menuitem.
But we could show:
Find               Ctrl+F
Find and Replace   Ctrl+H

even today and they would open the same dialog. But that could be strange, and maybe users would file bugs that there are duplicate items.

Comment 11

4 years ago
Comment on attachment 765065 [details] [diff] [review]
Patch

Review of attachment 765065 [details] [diff] [review]:
-----------------------------------------------------------------

r=mkmelin with the findrCmd.accesskey removed

I'm pretty sure this is wanted on linux. Gedit use it, also e.g. LibreOffice Writer use it.
Attachment #765065 - Flags: review?(mkmelin+mozilla) → review+
(Assignee)

Comment 12

4 years ago
Created attachment 766364 [details] [diff] [review]
Patch v2

carrying over review from mkmelin
Attachment #765065 - Attachment is obsolete: true
Attachment #765065 - Flags: ui-review?(bugs)
Attachment #766364 - Flags: review+
(Assignee)

Updated

4 years ago
Keywords: checkin-needed

Comment 13

4 years ago
The findReplaceCmd.* should be after all the findCmd.* entries. Can you fix it before checkin?
Keywords: checkin-needed
(Assignee)

Comment 14

4 years ago
Created attachment 766399 [details] [diff] [review]
Patch v2

Okay sir
Attachment #766364 - Attachment is obsolete: true
Attachment #766399 - Flags: feedback?(acelists)

Comment 15

4 years ago
Comment on attachment 766399 [details] [diff] [review]
Patch v2

Yes.
Attachment #766399 - Flags: feedback?(acelists) → feedback+

Updated

4 years ago
Keywords: checkin-needed
https://hg.mozilla.org/comm-central/rev/a478f425cfb6
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 24.0
Aceman, Suyash, Magnus, Blake - thanks! No matter how little, it's great to see things moving cooperatively and speedily like this! :)
Flags: needinfo?(bugs)
Whiteboard: [good first bug][has ui-review+]
(In reply to Thomas D. from comment #17)
> Aceman, Suyash, Magnus, Blake - thanks! No matter how little, it's great to
> see things moving cooperatively and speedily like this! :)

And Ryan not to forget for rapid rollouts! :)
I've also added the new shortcut to keyboard shortcuts documentation (1), which unfortunately already shows up now for TB17 in spite of correct {for} syntax which should only show up for TB24 and above (bug 886699):

|Find and Replace Text in Current Message||{for win,linux}{key Ctrl+F}{for tb24}<br>{key Ctrl+H}  &nbsp;''New in TB24''{/for}{/for}{for mac}{key Command+F}{/for}

But I think it's better to add it now a bit early (with indication), rather than forget about it later, and I don't want to remember and make plans for revisiting this later.

(1) https://support.mozillamessaging.com/en-US/kb/keyboard-shortcuts#w_writing-messages
Blocks: 713979
Blocks: 919276
Blocks: 998230
No longer blocks: 998230
You need to log in before you can comment on or make changes to this bug.