Closed Bug 256628 Opened 16 years ago Closed 5 years ago
change wording of Recheck Page button in spell checker
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...
hrm, just realized that this might have an impact on l10n, and might not make it for 1.0...
*** Bug 289404 has been marked as a duplicate of this bug. ***
Changes: -<!ENTITY recheckButton.label "Recheck Page"> -<!ENTITY recheckButton.accessKey "P"> +<!ENTITY recheckButton.label "Recheck Message"> +<!ENTITY recheckButton.accessKey "M">
Attachment #194967 - Flags: review?(mscott)
Comment on attachment 194967 [details] [diff] [review] patch this then makes the spell check dialog in editor say recheck message instead of page
Attachment #194967 - Flags: review?(mscott) → review-
please don't nominate things like this for stop ship bug status. Ask if you need help understanding what these flags mean. Thanks.
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.
Status: NEW → ASSIGNED
*** Bug 322194 has been marked as a duplicate of this bug. ***
(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?
Well I can't figure this out without help so removing myself as the assignee in hopes someone else can take this bug.
Assignee: supernova_00 → nobody
trivial patch. needs new owner
Whiteboard: [patchlove → [patchlove]
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.
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.
In the spirit of shortness, how about just "recheck" or restart?
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
Attachment #194967 - Attachment is obsolete: true
Kurt, any luck for a new patch?
No one has come up with a definitive answer on what to do here.
(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... :)
(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?
Component: Message Compose Window → Editor
Product: Thunderbird → Core
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 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
(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.
Component: Editor → Message Compose Window
Product: Core → Thunderbird
Version: unspecified → Trunk
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?
(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...
(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.
(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.
Magnus, ping for reviewing this 4-line string change patch?
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" ?
Attachment #8627000 - Flags: review?(mkmelin+mozilla) → review-
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?
(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 on attachment 8654521 [details] [diff] [review] Patch v. 3.1 - [Recheck Page] → [Recheck Text] Yeah, this is okay.
Attachment #8654521 - Flags: ui-review?(richard.marti) → ui-review+
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
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 43.0
You need to log in before you can comment on or make changes to this bug.