Change Folder Properties labels for incoming mail character encoding from "default" to "fallback" after bug 846221 introduced it for browser prefs

RESOLVED FIXED in Thunderbird 27.0

Status

defect
--
minor
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: rsx11m.pub, Assigned: rsx11m.pub)

Tracking

Trunk
Thunderbird 27.0

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

5.93 KB, patch
rsx11m.pub
: review+
bwinton
: ui-review+
Details | Diff | Splinter Review
Spun off per bug 902131 comment #6.

The term "default character encoding" for incoming messages isn't intuitive as it's actually a "fallback character encoding" if the message itself doesn't specify that encoding in its MIME headers. SeaMonkey bug 895751 and bug 902131 changed this for the browser and mail/news preference panes, respectively. I've filed bug 916817 for the Thunderbird preferences. This bug covers the Folder Properties labels in the shared code, which technically is part of the Account Manager.
Posted patch Proposed patch (obsolete) — Splinter Review
Changes "default" to "fallback" in the two occurrences of this dialog, along with the respective entity names.

Neil, I'm asking you for review rather than IanN as this is in the mailnews/ directory, with Blake covering the ui-r for Thunderbird. No API changes, thus no super-review appears to be required.

Note that I didn't include the "Used for legacy content that does not declare its encoding" label here. The tab is crowded already, and that information would be provided in the main preference dialog already. Also, with bug 916178 adding Help content for the Folder Properties dialog for SeaMonkey, this could be covered there in more detail.
Attachment #805364 - Flags: ui-review?(bwinton)
Attachment #805364 - Flags: review?(neil)
Comment on attachment 805364 [details] [diff] [review]
Proposed patch

>+<!ENTITY folderCharsetEnforce.label              "Apply fallback to all messages in the folder (individual message character encoding settings and auto-detection will be ignored)">
I'm not 100% convinced about this wording, as this checkbox changes the meaning of the encoding from a default to an override. At the very least I think it should say "encoding" instead of "fallback". r=me with that fixed, assuming bwinton agrees.
Attachment #805364 - Flags: review?(neil) → review+
I've changed "Apply fallback..." to "Apply encoding..." which sounds fine to me. I was thinking of making it "Apply this encoding..." instead to be more specific, but then the label wraps into a third row for me, thus removed it again. Anyway, it should be unambiguous which encoding is meant within the context of that dialog.
Attachment #805364 - Attachment is obsolete: true
Attachment #805364 - Flags: ui-review?(bwinton)
Attachment #807352 - Flags: ui-review?(bwinton)
Attachment #807352 - Flags: review+
Blake: Ping for review. This has string changes, thus I'd like to land it in the current cycle given that bug 895751 and bug 902131 have landed for SeaMonkey already with corresponding changes in our preference panes.
Comment on attachment 807352 [details] [diff] [review]
Proposed patch (v2)

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

Yeah, the folderCharsetEnforce.label still seems really, really long, but this is an improvement, so ui-r=me.

Thanks,
Blake.
Attachment #807352 - Flags: ui-review?(bwinton) → ui-review+
Great, thanks. Push for comm-central, please.
Keywords: checkin-needed
https://hg.mozilla.org/comm-central/rev/f769584a86da
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 27.0
Backed out for causing xpcshell failures.
https://hg.mozilla.org/comm-central/rev/48ea5d2497f4

https://tbpl.mozilla.org/php/getParsedLog.php?id=28939824&tree=Thunderbird-Trunk
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: Thunderbird 27.0 → ---
The test failures are related to authentication, hence nothing within the scope of this patch.

I don't see how any of these tests are using either folderCharsetTab.{label,accesskey} or folderCharsetOverride.{label,accesskey}, and I didn't change id="folderCharsetOverride" in the XUL code. Thus, the respective accesses to gMsgFolder.charsetOverride in folderProps.js should function as before.

But, let's wait for the tinderbox builds to finish, I would be surprised if this is the cause.
The bustage is coming from somewhere else. Re-landed.
https://hg.mozilla.org/comm-central/rev/c0ae5429efbe
Target Milestone: --- → Thunderbird 27.0
Closing this again as the bustage is unrelated (tracked in bug 925489).
Status: REOPENED → RESOLVED
Closed: 6 years ago6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.