Closed
Bug 256628
Opened 20 years ago
Closed 9 years ago
change wording of Recheck Page button in spell checker
Categories
(Thunderbird :: Message Compose Window, defect)
Thunderbird
Message Compose Window
Tracking
(Not tracked)
RESOLVED
FIXED
Thunderbird 43.0
People
(Reporter: bugzilla, Assigned: thomas8)
References
Details
(Whiteboard: [patchlove])
Attachments
(1 file, 4 obsolete files)
2.28 KB,
patch
|
neil
:
review+
Paenglab
:
ui-review+
|
Details | Diff | Splinter Review |
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...
Reporter | ||
Comment 1•20 years ago
|
||
hrm, just realized that this might have an impact on l10n, and might not make it for 1.0...
Comment 2•19 years ago
|
||
*** 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 4•19 years ago
|
||
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-
Comment 5•19 years ago
|
||
please don't nominate things like this for stop ship bug status. Ask if you need help understanding what these flags mean. Thanks.
Flags: blocking1.9a1?
Flags: blocking1.9a1-
Flags: blocking1.8b5?
Flags: blocking1.8b5-
Flags: blocking1.8b4?
Flags: blocking1.8b4-
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. ***
Comment 8•19 years ago
|
||
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
(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?
Updated•17 years ago
|
QA Contact: message-compose
Comment 10•17 years ago
|
||
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
Updated•16 years ago
|
Whiteboard: [patchlove
Comment 12•16 years ago
|
||
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•16 years ago
|
||
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•16 years ago
|
||
In the spirit of shortness, how about just "recheck" or restart?
Comment 15•15 years ago
|
||
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
Comment 16•15 years ago
|
||
Kurt, any luck for a new patch?
Comment 17•15 years ago
|
||
No one has come up with a definitive answer on what to do here.
Comment 18•15 years ago
|
||
(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... :)
Assignee | ||
Comment 19•9 years ago
|
||
(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?
Flags: needinfo?(vseerror)
Assignee | ||
Updated•9 years ago
|
Component: Message Compose Window → Editor
Product: Thunderbird → Core
Assignee | ||
Comment 21•9 years ago
|
||
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
Assignee: nobody → bugzilla2007
Status: NEW → ASSIGNED
Attachment #8626900 -
Flags: ui-review?(richard.marti)
Attachment #8626900 -
Flags: review?(mkmelin+mozilla)
Comment 22•9 years ago
|
||
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
Attachment #8626900 -
Flags: ui-review?(richard.marti)
Attachment #8626900 -
Flags: ui-review+
Attachment #8626900 -
Flags: review?(mkmelin+mozilla)
Assignee | ||
Comment 23•9 years ago
|
||
(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.
Attachment #8626900 -
Attachment is obsolete: true
Attachment #8627000 -
Flags: ui-review+
Attachment #8627000 -
Flags: review?(mkmelin+mozilla)
Updated•9 years ago
|
Component: Editor → Message Compose Window
Product: Core → Thunderbird
Version: unspecified → Trunk
Comment 24•9 years ago
|
||
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?
Assignee | ||
Comment 25•9 years ago
|
||
(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•9 years ago
|
||
(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.
Assignee | ||
Comment 27•9 years ago
|
||
(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.
Assignee | ||
Comment 28•9 years ago
|
||
Magnus, ping for reviewing this 4-line string change patch?
Flags: needinfo?(mkmelin+mozilla)
Comment 29•9 years ago
|
||
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-
Updated•9 years ago
|
Flags: needinfo?(mkmelin+mozilla)
Assignee | ||
Comment 30•9 years ago
|
||
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?
Attachment #8627000 -
Attachment is obsolete: true
Attachment #8654518 -
Flags: ui-review?(richard.marti)
Attachment #8654518 -
Flags: review?(neil)
Assignee | ||
Updated•9 years ago
|
Attachment #8654518 -
Flags: ui-review?(richard.marti)
Attachment #8654518 -
Flags: review?(neil)
Assignee | ||
Comment 31•9 years ago
|
||
(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?
Attachment #8654518 -
Attachment is obsolete: true
Attachment #8654521 -
Flags: ui-review?(richard.marti)
Attachment #8654521 -
Flags: review?(neil)
Comment 32•9 years ago
|
||
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+
Updated•9 years ago
|
Attachment #8654521 -
Flags: review?(neil) → review+
Assignee | ||
Updated•9 years ago
|
Keywords: checkin-needed
Comment 33•9 years ago
|
||
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
Updated•9 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 43.0
You need to log in
before you can comment on or make changes to this bug.
Description
•