Last Comment Bug 256628 - change wording of Recheck Page button in spell checker
: change wording of Recheck Page button in spell checker
Status: RESOLVED FIXED
[patchlove]
:
Product: Thunderbird
Classification: Client Software
Component: Message Compose Window (show other bugs)
: Trunk
: All All
-- trivial (vote)
: Thunderbird 43.0
Assigned To: Thomas D. (currently busy elsewhere; needinfo?me)
:
:
Mentors:
: 289404 322194 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2004-08-23 13:16 PDT by sairuh (rarely reading bugmail)
Modified: 2015-09-07 02:44 PDT (History)
16 users (show)
mscott: blocking1.8b4-
mscott: blocking1.8b5-
mscott: blocking1.9a1-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
patch (1.30 KB, patch)
2005-09-05 16:06 PDT, u88484
mscott: review-
Details | Diff | Splinter Review
18064.patch v.2: change Recheck Page label to Recheck All (1.39 KB, patch)
2015-06-27 07:40 PDT, Thomas D. (currently busy elsewhere; needinfo?me)
richard.marti: ui‑review+
Details | Diff | Splinter Review
patch v.2.1 (2.23 KB, patch)
2015-06-28 06:13 PDT, Thomas D. (currently busy elsewhere; needinfo?me)
mkmelin+mozilla: review-
bugzilla2007: ui‑review+
Details | Diff | Splinter Review
Patch v. 3 - Recheck Text (1.51 KB, patch)
2015-08-29 08:55 PDT, Thomas D. (currently busy elsewhere; needinfo?me)
no flags Details | Diff | Splinter Review
Patch v. 3.1 - [Recheck Page] → [Recheck Text] (2.28 KB, patch)
2015-08-29 09:33 PDT, Thomas D. (currently busy elsewhere; needinfo?me)
neil: review+
richard.marti: ui‑review+
Details | Diff | Splinter Review

Description User image sairuh (rarely reading bugmail) 2004-08-23 13:16:05 PDT
tested with 200408230x-0.8 on linux and mac; couldn't find an existing bug
(either tbird or moz mail) for this.

in the Spell Check dialog for thunderbird, shouldn't the Recheck Page button be
something like Recheck Message instead? I imagine this dialog was borrowed from
mozilla, so Recheck Page was from the composer (web page editor) app...
Comment 1 User image sairuh (rarely reading bugmail) 2004-08-23 13:27:34 PDT
hrm, just realized that this might have an impact on l10n, and might not make it
for 1.0...
Comment 2 User image Magnus Melin 2005-08-26 10:11:11 PDT
*** Bug 289404 has been marked as a duplicate of this bug. ***
Comment 3 User image u88484 2005-09-05 16:06:57 PDT
Created attachment 194967 [details] [diff] [review]
patch

Changes:

-<!ENTITY recheckButton.label "Recheck Page">
-<!ENTITY recheckButton.accessKey "P">
+<!ENTITY recheckButton.label "Recheck Message">
+<!ENTITY recheckButton.accessKey "M">
Comment 4 User image Scott MacGregor 2005-09-05 21:33:27 PDT
Comment on attachment 194967 [details] [diff] [review]
patch

this then makes the spell check dialog in editor say recheck message instead of
page
Comment 5 User image Scott MacGregor 2005-09-05 22:30:03 PDT
please don't nominate things like this for stop ship bug status. Ask if you need
help understanding what these flags mean. Thanks.
Comment 6 User image u88484 2005-09-06 06:00:10 PDT
Scott can you point me to the right file then because I searched and searched
with  lxr and that is the only place 'Recheck Page' is found. 
Comment 7 User image u88484 2006-01-03 07:23:54 PST
*** Bug 322194 has been marked as a duplicate of this bug. ***
Comment 9 User image u88484 2006-05-18 16:34:20 PDT
(In reply to comment #8)
> http://lxr.mozilla.org/seamonkey/source/editor/ui/locales/en-US/chrome/dialogs/EditorSpellCheck.dtd#64
> which is used in
> http://lxr.mozilla.org/seamonkey/source/editor/ui/dialogs/content/EdSpellCheck.xul#73
> 

That would still change it in editor.

Mscott, is there anyway to change this without also changing in editor? Like an ifdef or something?
Comment 10 User image u88484 2007-09-20 03:49:14 PDT
Well I can't figure this out without help so removing myself as the assignee in hopes someone else can take this bug.
Comment 11 User image Wayne Mery (:wsmwk, NI for questions) 2008-08-28 20:26:27 PDT
trivial patch. needs new owner
Comment 12 User image Steve Simms 2008-09-03 11:01:02 PDT
Looking through the comments and the patch, it looks like the same button (or dialog?) is being used in two places, at least in SeaMonkey.

Is this also the case for Thunderbird?  If not, this patch will just need to get refreshed, and otherwise appears to be fine.
Comment 13 User image Phil Ringnalda (:philor) 2008-09-03 12:10:07 PDT
What comment 4 through comment 9 are saying is that this dialog uses exactly the same XUL and DTD in SeaMonkey's Composer, SeaMonkey's mail, and Thunderbird. Because the answer to comment 9 is "no, you can't #ifdef in a localized file" fixing it either requires forking the DTD file to change one word, which I hope we wouldn't accept a patch to do, or rephrasing it to something like "recheck document" which will work whether the document is an email or an HTML page.
Comment 14 User image Wayne Mery (:wsmwk, NI for questions) 2008-09-09 03:35:12 PDT
In the spirit of shortness, how about just "recheck" or restart?
Comment 15 User image Gary Kwong [:gkw] [:nth10sd] 2009-03-15 03:25:56 PDT
Comment on attachment 194967 [details] [diff] [review]
patch

Patch has bitrotted.

$ patch --dry-run < ~/Desktop/tbTestPatches/256628.diff 
(Stripping trailing CRs from patch.)
patching file EditorSpellCheck.dtd
Hunk #1 FAILED at 56.
1 out of 1 hunk FAILED -- saving rejects to file EditorSpellCheck.dtd.rej
Comment 16 User image Gary Kwong [:gkw] [:nth10sd] 2009-06-17 10:22:11 PDT
Kurt, any luck for a new patch?
Comment 17 User image u88484 2009-06-17 11:33:57 PDT
No one has come up with a definitive answer on what to do here.
Comment 18 User image Gary Kwong [:gkw] [:nth10sd] 2009-06-17 17:41:57 PDT
(In reply to comment #17)
> No one has come up with a definitive answer on what to do here.

Getting our UI guru on board... :)
Comment 19 User image Thomas D. (currently busy elsewhere; needinfo?me) 2015-06-27 01:53:54 PDT
(In reply to Wayne Mery (:wsmwk, use Needinfo for questions) from comment #14)))
> In the spirit of shortness, how about just "recheck" or restart?

Comment 13 suggests "Recheck document". I'd agree with Wayne's implied statement that this might be a little too long, more so in other languages where "recheck" is already longer than EN, e.g. DE:
("Recheck Page")
 "Seite erneut prüfen"
 "Dokument erneut prüfen"
("Recheck Document")

Just "Recheck" might not be clear enough about the scope of rechecking - will it just recheck the word in the input box which is very close? Given that rechecking will actually dump all your previous judgements about ignoring words and what not, I think this should be a case of ux-error-prevention.

As a compromise (hoping it will turn out a bit shorter in most languages), what about this?
 "Recheck all"       <- my suggestion (EN)
("Recheck document") <- for comparison

 "Alles erneut prüfen"     <- my suggestion (DE)
("Dokument erneut prüfen") <- for comparison

Wayne?
Comment 20 User image Wayne Mery (:wsmwk, NI for questions) 2015-06-27 03:20:45 PDT
Yes
Comment 21 User image Thomas D. (currently busy elsewhere; needinfo?me) 2015-06-27 07:40:01 PDT
Created attachment 8626900 [details] [diff] [review]
18064.patch v.2: change Recheck Page label to Recheck All

Suggested reviewer Ehsan doesn't do reviews, so I'll pick mkmelin. Pls adjust as needed.

Richard, pls see comment 19 for ui-review, to which Wayne agreed in comment 20.
We're changing the label not just for TB, but for any consumers of Editor, that's why we chose a short and neutral one-fits-all label.

Access keys (for EN-US)

+<!ENTITY recheckButton2.label "Recheck All">
- all potential access keys are currently taken, and make sense where they are
- duplicate access keys are unwanted
- so we need to steal one from a less frequently used element:

-<!ENTITY checkwordButton.accessKey "k">
+<!ENTITY checkwordButton.accessKey "W">
(no need to change identifier because this secondary change is only applicable for EN-US where we are changing it just now; other locales will need to juggle around with their own access keys so there's no need to notify anyone about this change)

[Check Word] had "k", now "W" - this is for the imo rare scenario when you replace a word with a totally different word during spell-check, AND want to spell-check the new word right here in the box

> +<!ENTITY recheckButton2.accessKey "k">
[Recheck All] now has "k" and identifiers updated to notify localizers
Comment 22 User image Richard Marti (:Paenglab) 2015-06-27 10:15:10 PDT
Comment on attachment 8626900 [details] [diff] [review]
18064.patch v.2: change Recheck Page label to Recheck All

ui-+ but this doesn't work. You have only changed the entities in the language file but you need also to change the XUL file which uses the entities. See https://dxr.mozilla.org/comm-central/search?q=recheckButton.label
Comment 23 User image Thomas D. (currently busy elsewhere; needinfo?me) 2015-06-28 06:13:41 PDT
Created attachment 8627000 [details] [diff] [review]
patch v.2.1

(In reply to Richard Marti (:Paenglab) from comment #22)
> Comment on attachment 8626900 [details] [diff] [review]
> 18064.patch v.2: change Recheck Page label to Recheck All
> 
> ui-+ but this doesn't work. You have only changed the entities in the
> language file but you need also to change the XUL file which uses the
> entities. See
> https://dxr.mozilla.org/comm-central/search?q=recheckButton.label

Of course, thanks.
Comment 24 User image Jorg K (GMT+1) 2015-06-28 13:24:37 PDT
Please note the following:
This is *not* an M-C core editor bug.
The source files clearly live in C-C (https://dxr.mozilla.org/comm-central/source/editor/ui).
Ehsan is an M-C developer, he doesn't do C-C reviews.

Why did you change recheckButton.label to recheckButton2.label? Why not just change the value?
Comment 25 User image Thomas D. (currently busy elsewhere; needinfo?me) 2015-06-28 14:41:15 PDT
(In reply to Jorg K from comment #24)
> Please note the following:
> This is *not* an M-C core editor bug.
> The source files clearly live in C-C

Yeah, thanks! I'm glad to be corrected and learn more, as I had my doubts after I set it...

> (https://dxr.mozilla.org/comm-central/source/editor/ui).
> Ehsan is an M-C developer, he doesn't do C-C reviews.

Well, I didn't ask him anyway and obviously reviewers differ according to products/components. Fwiw, Ehsan according to his current BMO self-description does not do *any* reviews atm, perhaps because his review queue is already long.
>  Ehsan Akhgari (not reviewing patches, not reading bugmail, needinfo? me!)

> Why did you change recheckButton.label to recheckButton2.label? Why not just
> change the value?

Tradition has it that developers claim it's necessary to change the identifier/name when we change the value of such entities, supposedly because it's said to be the only reliable way of notifying localizers about the need for a new translation. I totally agree with Jorg and I'm on record that this practice is unexpected; in fact, it's very bad as it should never be necessary to change the name of the entity when we're just tweaking the value. If this nasty tradition is still needed, it's really bad design in the localisation system. Jorg, you'll be hero if you offer evidence that this is no longer needed, or rather, if you make the responsible folks adapt the localisation system to work without this unbelievable hack...
Comment 26 User image Jorg K (GMT+1) 2015-06-28 15:02:51 PDT
(In reply to Thomas D. from comment #25)
> ... evidence that this is no longer needed, ...

I know nothing about localisation, so I leave it to Magnus and his review.

Sadly I read here:
https://developer.mozilla.org/en-US/docs/Mozilla/Localization/Localization_content_best_practices
"Changing the string ID ... is the only reliable method to ensure that localizers update existing localizations."

Since the change is rather minor, it wouldn't be a drama if one translation were missed.
Comment 27 User image Thomas D. (currently busy elsewhere; needinfo?me) 2015-06-29 00:13:20 PDT
(In reply to Jorg K from comment #26)
> https://developer.mozilla.org/en-US/docs/Mozilla/Localization/
> Localization_content_best_practices

Thanks for the link to documentation!

> "Changing the string ID ... is the only reliable method to ensure that
> localizers update existing localizations."

Sad indeed.

> Since the change is rather minor, it wouldn't be a drama if one translation
> were missed.

Minor or not, we're changing this for a reason, and we want the new strings in every translation, so until there's a better universal system for localization, we'll stick to the procedure as a matter of principle and product quality. Also to avoid further duplicates of this bug filed against other localizations who missed the string update because we didn't alert them via ID change.
Comment 28 User image Thomas D. (currently busy elsewhere; needinfo?me) 2015-08-11 02:28:29 PDT
Magnus, ping for reviewing this 4-line string change patch?
Comment 29 User image Magnus Melin 2015-08-13 12:16:20 PDT
Comment on attachment 8627000 [details] [diff] [review]
patch v.2.1

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

I'm not a reviewer for editor/. Try neil ...

And yes, when we change meanings we have to update the localization key in the entity file. Nobody likes it much but it's the only bulletproof way anyone's come up with so far.

::: editor/ui/locales/en-US/chrome/dialogs/EditorSpellCheck.dtd
@@ +9,4 @@
>  <!ENTITY wordEditField.label "Replace with:">
>  <!ENTITY wordEditField.accessKey "w">
>  <!ENTITY checkwordButton.label "Check Word">
> +<!ENTITY checkwordButton.accessKey "W">

w is already taken (replace _w_ith) two lines above

@@ +28,4 @@
>  <!ENTITY addToUserDictionaryButton.accessKey "d">
>  <!ENTITY editUserDictionaryButton.label "Edit…">
>  <!ENTITY editUserDictionaryButton.accessKey "E">
> +<!ENTITY recheckButton2.label "Recheck All">

I'm not sure this is any cleared
How about "Recheck Text" ?
Comment 30 User image Thomas D. (currently busy elsewhere; needinfo?me) 2015-08-29 08:55:03 PDT
Created attachment 8654518 [details] [diff] [review]
Patch v. 3 - Recheck Text

Hi Neil (suggested reviewer per Magnus comment 29), a one-line string fix patch for your review, tia :)

Richard, "Recheck Text" per Magnus comment 29 sounds good, clear and sufficiently neutral to me, and you?
Comment 31 User image Thomas D. (currently busy elsewhere; needinfo?me) 2015-08-29 09:33:39 PDT
Created attachment 8654521 [details] [diff] [review]
Patch v. 3.1 - [Recheck Page] → [Recheck Text]

(sorry, last patch was incomplete)

Hi Neil (suggested reviewer per Magnus comment 29), a one-line string fix patch for your review, tia :)

Richard, "Recheck Text" per Magnus comment 29 sounds good, clear and sufficiently neutral to me, and you?
Comment 32 User image Richard Marti (:Paenglab) 2015-08-29 09:53:29 PDT
Comment on attachment 8654521 [details] [diff] [review]
Patch v. 3.1 - [Recheck Page] → [Recheck Text]

Yeah, this is okay.
Comment 33 User image aleth [:aleth] 2015-09-07 02:41:25 PDT
https://hg.mozilla.org/comm-central/rev/a5e9eff60b9cc5350c5f4d6b566eef885b1accdf
Bug 256628 - Change "Recheck Page" button label to "Recheck Text" in spell check dialog. r=neil,ui-r=paenglab

Note You need to log in before you can comment on or make changes to this bug.