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

Status: RESOLVED FIXED

(Thunderbird :: Message Compose Window, enhancement)

Description

[Thomas D. (:thomas8)]

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

Comment 1

[Mark Banner (:standard8)]

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.

Comment 2

[Thomas D. (:thomas8)]

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

Comment 3

[Blake Winton (:bwinton) (:☕️)]

Sounds good to me for Windows.  For Linux I'ld like you to run it by Andreas first.

Thanks,
Blake.

Comment 4

[Thomas D. (:thomas8)]

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     }

Comment 5

[:aceman]

So you actually propose to open the same Find&Replace dialog when pressing any of Ctrl+F or Ctrl+H ?

Comment 6

[Thomas D. (:thomas8)]

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

Comment 7

[Suyash Agarwal (:sshagarwal)]

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

Comment 8

[:aceman]

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.

Comment 9

[Thomas D. (:thomas8)]

(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

[:aceman]

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

[Magnus Melin [:mkmelin]]

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.

Comment 12

[Suyash Agarwal (:sshagarwal)]

Attachment: Patch v2

carrying over review from mkmelin

Comment 13

[:aceman]

The findReplaceCmd.* should be after all the findCmd.* entries. Can you fix it before checkin?

Comment 14

[Suyash Agarwal (:sshagarwal)]

Attachment: Patch v2

Okay sir

Comment 15

[:aceman]

Comment on attachment 766399 [details] [diff] [review]
Patch v2

Yes.

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/comm-central/rev/a478f425cfb6

Comment 17

[Thomas D. (:thomas8)]

Aceman, Suyash, Magnus, Blake - thanks! No matter how little, it's great to see things moving cooperatively and speedily like this! :)

Comment 18

[Thomas D. (:thomas8)]

(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! :)

Comment 19

[Thomas D. (:thomas8)]

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