1081693 - Port Bug 947507 to thunderbird: Reset intl.charset.detector if not set to off, Japanese, Russian or Ukrainian

Status: RESOLVED FIXED

(Thunderbird :: General, defect)

Description

[ISHIKAWA, Chiaki]

Hi,

I am a user of TB and noticed that
with the current C-C TB, which would be released in the future,
Japanese, Russian and Ukrainian users may
have a chance of seeing plain text attachment files
shown in garbled manner when they turn on the view the 
attachment inline feature.
(This is because there are a few different encodings of 
often used characters in the regions, and
because "universal charset detector" (or whatever) has been
removed from C-C, the charset of attachment is not recognized automatically any more by default. Thus a possible garbled display of plain text attachments viewed as inline.

According to
https://bugzilla.mozilla.org/show_bug.cgi?id=715823#c37
and subsequent comments,
this garbling indeed happens and
as a Japanese e-mail client user,
I needed to set the
preference
intl.charset.detector to
ja_parallel_state_machine 
as mentioned in the following paragraph.
It looks the following is the value that must be defined:
Japanese users  - ja_parallel_state_machine
Russian users   -  ruprob
Ukrainian users -  ukprob
everybody else  -  default (blank)

These values are from:

> The value is set in localization.
http://hg.mozilla.org/l10n-central/ja/file/6ce31c3a1c68/toolkit/chrome/global/intl.properties#l49

    43 # LOCALIZATION NOTE (intl.charset.detector):
    44 # This preference controls the initial setting for the character encoding
    45 # detector. Valid values are ja_parallel_state_machine for Japanese, ruprob
    46 # for Russian and ukprob for Ukrainian and the empty string to turn detection
    47 # off. The value must be empty for locales other than Japanese, Russian and
    48 # Ukrainian.
    49 intl.charset.detector			= ja_parallel_state_machine
    50 
 
Ordinary users in the affected regions
will not understand that the removal of
"International charset detector" 
means that they may occasionally see garbled inline view of
plain text attachments in the future.

So we should mention this clearly in the upcoming release note
else there will be MANY support calls regarding
garbled inline display of attachments.
To avoid the support cost,
this manual action should be clearly spelled out
in the release document, and this ought to be
near the top of web page announcement, too!

TIA

Comment 1

[Henri Sivonen (:hsivonen)]

Better not to bother users about it and to auto-migrate the pref like was done in Firefox:
https://hg.mozilla.org/mozilla-central/rev/dd19bec07e82

Comment 2

[ISHIKAWA, Chiaki]

(In reply to Henri Sivonen (:hsivonen) from comment #1)
> Better not to bother users about it and to auto-migrate the pref like was
> done in Firefox:
> https://hg.mozilla.org/mozilla-central/rev/dd19bec07e82

I see the relevant code:


    1.11    _migrateUI: function BG__migrateUI() {
    1.12 -    const UI_VERSION = 18;
    1.13 +    const UI_VERSION = 19;
    1.14      const BROWSER_DOCURL = "chrome://browser/content/browser.xul#";
    1.15      let currentUIVersion = 0;
    1.16      try {
    1.17        currentUIVersion = Services.prefs.getIntPref("browser.migration.version");
    1.18      } catch(ex) {}
    1.19      if (currentUIVersion >= UI_VERSION)
    1.20        return;
    1.21  
    1.22 @@ -1531,16 +1531,32 @@ BrowserGlue.prototype = {
    1.23            let toolbar = this._rdf.GetResource(BROWSER_DOCURL + toolbarId);
    1.24            if (this._getPersist(toolbar, resource)) {
    1.25              this._setPersist(toolbar, resource);
    1.26            }
    1.27          }
    1.28        }
    1.29      }
    1.30  
    1.31 +    if (currentUIVersion < 19) {
    1.32 +      let detector = null;    
    1.33 +      try {
    1.34 +        detector = Services.prefs.getComplexValue("intl.charset.detector",
    1.35 +                                                  Ci.nsIPrefLocalizedString).data;
    1.36 +      } catch (ex) {}
    1.37 +      if (!(detector == "" ||
    1.38 +            detector == "ja_parallel_state_machine" ||
    1.39 +            detector == "ruprob" ||
    1.40 +            detector == "ukprob")) {
    1.41 +        // If the encoding detector pref value is not reachable from the UI,
    1.42 +        // reset to default (varies by localization).
    1.43 +        Services.prefs.clearUserPref("intl.charset.detector");
    1.44 +      }
    1.45 +    }
    1.46 +

I looked for *Glue.js
and found 
mail/components/mailGlue.js

but it did not have any code like this.
Instead I found a reference to 
  _onProfileStartup: function MailGlue__onProfileStartup() {
    TBDistCustomizer.applyPrefDefaults();

    // handle any migration work that has to happen at profile startup
    MailMigrator.migrateAtProfileStartup();   <==== THIS!

    // check if we're in safe mode
    if (Services.appinfo.inSafeMode) {
      Services.ww.openWindow(null, "chrome://messenger/content/safeMode.xul",
                             "_blank", "chrome,centerscreen,modal,resizable=no", null);
    }
  },

and found
mail/base/modules/mailMigrator.js

In it, I found a function that started with

  /**
   * Determine if the UI has been upgraded in a way that requires us to reset
   * some user configuration.  If so, performs the resets.
   */
  _migrateUI: function MailMigrator__migrateUI() {
    // The code for this was ported from
    // mozilla/browser/components/nsBrowserGlue.js
    const UI_VERSION = 5;
    const MESSENGER_DOCURL = "chrome://messenger/content/messenger.xul#";
    const UI_VERSION_PREF = "mail.ui-rdf.version";
    let currentUIVersion = 0;

    try {

This, I suppose, should be changed.
But 
what would be
UI_VERSION
to start with?

I better leave this someone in the know. This bugzilla should be a reminder for the eventual patch before 
the next release. 

Is there special keyword or something that needs to be set for this purpose?

Comment 3

[Henri Sivonen (:hsivonen)]

(In reply to ISHIKAWA, Chiaki from comment #2)
> This, I suppose, should be changed.

Yes.

> But 
> what would be
> UI_VERSION
> to start with?

It's incremented by one every time a new migration behavior is added to the function, so since it's now 5, you'd change it to 6.

> Is there special keyword or something that needs to be set for this purpose?

I guess the tracking-thunderbird35 flag is the closest thing available.

Comment 4

[ISHIKAWA, Chiaki]

(In reply to Henri Sivonen (:hsivonen) from comment #3)
> (In reply to ISHIKAWA, Chiaki from comment #2)
> > This, I suppose, should be changed.
> 
> Yes.
> 
> > But 
> > what would be
> > UI_VERSION
> > to start with?
> 
> It's incremented by one every time a new migration behavior is added to the
> function, so since it's now 5, you'd change it to 6.
> 
> > Is there special keyword or something that needs to be set for this purpose?
> 
> I guess the tracking-thunderbird35 flag is the closest thing available.

Thank you for the information.

Someone in the know should be able to fix this part, I hope :-)

Also, thank you for putting in the flag.

If nobody steps in and I have finished debugging some issues like 
Bug 837205 and have still time left from some exhibition stuff for my day job,
I might come back to this to hack on the code.

The day job is where reliable bug-free thunderbird is required.

Toward the end of the year, I oversee a production of a technical report of a sort
for the last dozen years or so. I checked and found that on the average about 1000 e-mails, many with attachment (mostly PDF files for proofreading, etc., some as large as 5MB) got delivered in my mailbox each month (November alone). So in a sense, October is the last month of the year for me to contribute to TB debugging efforts
before I use TB under heavy load in November and December: during such heavy usage,I found out issues like the overflowing folder size, saving attachment to write-protected directory silently failed [very bad for me, fixed finally in the last 12 months or so], compaction in almost full file system ate my folder [very bad, but it was in V2 -> V3 range. Hopefully fixed now], etc. So I get to use debugged TB release,  the result of community's labor in the next couple of months.

TIA

Comment 5

[Magnus Melin [:mkmelin]]

Attachment: bug1081693_reset_intl.charset.detector.patch

Port https://hg.mozilla.org/mozilla-central/rev/dd19bec07e82

Comment 6

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

Comment on attachment 8530675 [details] [diff] [review]
bug1081693_reset_intl.charset.detector.patch

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

Yep, looks like a clean port!  Thanks!  :)

Comment 7

[Magnus Melin [:mkmelin]]

https://hg.mozilla.org/comm-central/rev/a46f0d8be50b -> FIXED