530621 - Find and Replace: Replace button appears broken, should find next occurence when there's no selection

Status: ASSIGNED

(Thunderbird :: Message Compose Window, defect)

Description

[Thomas D. (:thomas8)]

STR

1 compose msg with "test asdf test asdf test asdf"
2 ctrl+f -> find & replace dialogue, find: test, replace:***
3a click replace
3b click find next (will highlight next occurence), then replace button several times

actual

3a nothing
3b will only replace first occurence, subsequent replace clicks do nothing

expected

3a
1st click on replace should find and highlight next (1st) occurence of find:
2nd click on replace should replace highlighted (1st) occurence (just as it does now, i.e. without advancing to the next occurence, because that's the other button, "replace&find"); then continue with 3b

3b
another click on replace will again find&highlight the next (2nd) occurence
another click on replace will replace highlighted (2nd) occurence
etc.

This may sound weird in theory, but a two-click advance and replace is definitely a lot better than the current non-responsive behaviour. Actually we just advance to next find rather than doing nothing when user clicks replace again. We can't automatically advance to the next find because we have a separate button for that, "Replace and Find". But if user insists on clicking replace again after we've successfully replaced without advancing, on each second click we should find & highlight the next occurence so that user can replace it.

The advantage of this seemingly strange method is complete (visual) control of find and replace: On 1st click, you can see what's going to be replaced, the 2nd click replaces but you can still see the same spot to check how it looks after the replace. It can actually give you peace of mind for delicate replacements. You can do the same if you alternately click Find, then replace, then Find, and replace again, which is obviously much more complicated than what I propose here. But it would actually still work (as it does now) after implementing this enhancement, so we're not taking any functionality away.

Only other alternative I see would be disabling replace when there's nothing highlighted, which just doesn't seem right. We actually want to do a good thing here by having two separate buttons (and it IS a good thing), but unfortunately most other programs don't have two buttons and users will just click on "replace" and the thing will replace and advance. So we should find a balance between that concept and our concept. Buttons that don't do anything on click are no good.

Comment 1

[Ludovic Hirlimann [:Usul]]

Sounds like a nice Core:Editor bug ...

Comment 4

[Thomas D. (:thomas8)]

So in a nutshell, the current implementation is designed to offer two flavors of replacing:

1) Replace (*without* advancing to find next occurence) [nice feature, but currently broken and unintuitive, this bug]

2) Replace and Find (replaces currently selected occurence and immediately advances to select next occurence) [this works as designed; confusingly, in major word processors like MS Word this option and behaviour is just labelled "Replace").

1) Replace (*without* advancing to find next occurence) [nice feature, but currently broken and unintuitive, this bug]
- advantage: user can verify the replaced text *after* replacing (to check layout, coherence etc.)
- this can be designed in two ways:

  a) 2-step replacement (recommended in comment 0)
  - every 1st click finds & selects next occurence (so that you can see what you are replacing *before* replacing it: that's the missing bit in current implementation.
  - every 2nd click just replaces without advancing (so that you can check how it looks *after* replacing)
  So this method gives the user a maximum of control for sensitive replacments.

  b) 1-step replacement (possible, but less recommended; very similar to method 2) below, but less transparent and more error-prone)
  - every 1st click finds and immediately replaces the next occurence without advancing (so that you can never see the original text, but you can still verify each replacement after the fact)
  Method b) has a high potential for confusion because you never see anything selected so it's hard to see what has actually changed and where (although this works with charming perfection and speed if there's *no dialogue* so cursor can show the current position after each replacement).

2) Replace and Find (finds next occurence immediately after replacing previous occurence) [this works as designed; confusingly, this option is just called "Replace" in major word processors like MS Word]
- this is a good compromise between speed and control: reasonable control as users can see the original text found before replacing (assuming they know their 'replace-with' text which is always the same); higher speed because it's only 1 click for each replacement.

Comment 0 or this comment might be a hard read but the irritation is just because of habit formation where we learned in other apps like MS Word that "Replace" does what TB has more correctly labelled "Replace and Find", and TB's additional option of plain "Replace" (without advancing) doesn't exist in those apps (although it's very useful), and it's currently broken in TB.

With that caveat, I can still 100% subscribe to what I've suggested in comment 0.
Perhaps, for external consistency, we could change labels:

"Replace" could be labelled "Replace and verify" (or something similar to express that users can double-check *after* replacing).

"Replace and Find" could be labelled "Replace" to match with other applications. I think that's what most people are used to as "plain vanilla replacing".

Comment 5

[binkie]

Nobody needs "Replace and Verify" because most people verify anyway, as they go along. The button should be labeled "Find and Replace" like everybody else's, not "Replace and Find." With "Find and Replace" (if it actually DOES that), one can pause to verify – or not.

Clicking twice to accomplish what everyone else accomplishes with ONE click is lame. Plus it is not at all obvious that one would need to do this. Hence the title of this page "Replace button appears broken!"

Comment 6

[Thomas D. (:thomas8)]

Attachment: 530621.responsiveReplaceButton.patch

Let's make the Replace button more responsive, while preserving full benefits of the current intentional design!

Aceman, I think it will be alright if you'll review this one-line patch; as for ui-review, I was encouraged to just set them myself so basically, this already has my ui-r+, but it's always good to hear a 2nd opinion :)

I'm taking a conservative approach here which just fixes what feels broken while preserving the full functionality currently found in this dialogue.
I recommend just testing this in action, which feels much better than my clumsy explanations. Ideally test on a very long message (150+ lines) having "foo1", "foo2", and "foo3" respectively at begin, middle, and end of msg. Replace with "bar".

Current behaviour:

* [Replace] button = Replace Selection without finding the next occurence;
but if there's no selection yet, do nothing.
(+) user can verify the replaced version to see if the outcome is right :)
(-) Doing nothing and not responding on an enabled button confuses users, as seen on duplicates :(
(-) For repetitive single replacements, user is forced to alternately click [Find Next] and [Replace] button, which is very annoying and error-prone.
(2 clicks to replace one and move to the next)

Fyi (not changed by this bug), there's an alternative button in the dialogue:
* [Replace and Find] button = Replace Selection and immediately find next occurence
(+) single-click fast track replacing; this is the method used in most word processors
(-) user cannot easily verify the replaced version, especially if next occurence is further down beyond visible screen height (so replaced occurence is immediately scrolled out of view); depending on scenario, you might never see any occurences in their replaced version.

New Behaviour:

* [Replace] button = Replace Selection without finding the next occurence;
if there's no selection yet ([Find Next] has not yet been used), and user clicks [Replace] straight away (or again), helpfully find the next occurence before replacing.
(+) [Replace] button now always responsive
(+) we still allow user to verify the replacement
(+) repetitive, fully controlled single replacements now much more convenient, just keep clicking or pressing Enter on [Replace] button, which will automatically alternate between Find and Replace.
(For fully controlled replacing, we still need 2 clicks/keystrokes for each Find and Replace, but much more convenient now as click/keyboard target remains the same).

* [Replace and Find] button for fast-track single-click replacements (unchanged, see above)
Anybody who considers 2 clicks/keystrokes for each Find and Replace too much, and is happy with less control, can still just use this (other) button.

Comment 7

[Thomas D. (:thomas8)]

>        <button id="replace" label="&replaceButton.label;" accesskey="&replaceButton.accesskey;" 
> -          oncommand="onReplace();"/>
> +          oncommand="if (!onReplace()) onFindNext();"/>

I deliberately did NOT write a separate oncommand function for this because it imo it would just obscure things more, and this looks quite consistent with the other oncommand of the [Replace and Find] button

>        <button id="replaceAndFind" label="&replaceAndFindButton.label;" [Snip...]
>            oncommand="onReplace(); onFindNext();"/>

Comment 8

[:aceman]

Comment on attachment 8527319 [details] [diff] [review]
530621.responsiveReplaceButton.patch

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

Interesting idea. The code seems to work for me.
But as this is in /editor, we should call in a real reviewer.

Comment 9

[[e-mail]]

Comment on attachment 8527319 [details] [diff] [review]
530621.responsiveReplaceButton.patch

Seems to be a reasonable enhancement.

Comment 10

[Josiah Bruner [:jsbruner]]

Comment on attachment 8527319 [details] [diff] [review]
530621.responsiveReplaceButton.patch

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

So, I'm not totally sold here. I'm not against your suggestion necessarily, but I don't think it's the right change to make. Perhaps less because it's the wrong way to do things, but because it's redundant.

I looked at three (well-known and frequently-used) text editors to see how they handle things. I basically ended up with two kinds of interaction.

In cases where three buttons existed (Replace, Replace and Find, and Replace All), similar to ours, the interaction is how ours currently is. Replace did *not* search for the next index ever, Replace and Find did. TextMate was what I looked at.

Now, two other editors (LibreOffice and TextEdit), instead opted to use a two button approach, Replace and Replace All. In that case, Replace was really Replace and Find.

I understand what your proposal does is not *exactly* like Replace and Find, but I still think it's kind of confusing and redundant.

Therefore, I have two suggestions.

A) Keep things as-is, aka, like TextMate.
B) (The better solution). Make the Replace button do exactly what the Replace and Find button does, and then kill Replace and Find. That simplifies the entire interface, is easier to use, and we don't actually lose a practical use-case.

Sorry for the delayed response, I've been *very* busy lately, but it seems everyone is these days. :)

Comment 11

[binkie]

I like this from Thomas D: "On 1st click, you can see what's going to be replaced, the 2nd click replaces but you can still see the same spot to check how it looks after the replace. It can actually give you peace of mind for delicate replacements."

Comment 12

[faris18omar]

Has this been fixed ?