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

Status: RESOLVED FIXED

(MailNews Core :: Account Manager, defect)

Description

[rsx11m]

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.

Comment 1

[rsx11m]

Attachment: Proposed patch

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.

Comment 2

[[e-mail]]

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.

Comment 3

[rsx11m]

Attachment: Proposed patch (v2)

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.

Comment 4

[rsx11m]

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 5

[Blake Winton (:bwinton) (:☕️)]

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.

Comment 6

[rsx11m]

Great, thanks. Push for comm-central, please.

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/comm-central/rev/f769584a86da

Comment 8

[Ryan VanderMeulen [:RyanVM]]

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

Comment 9

[rsx11m]

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.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

The bustage is coming from somewhere else. Re-landed.
https://hg.mozilla.org/comm-central/rev/c0ae5429efbe

Comment 11

[rsx11m]

Closing this again as the bustage is unrelated (tracked in bug 925489).