Outlook/Hotmail trash folder is called "Deleted" in English and localised versions of Thunderbird

ASSIGNED
Assigned to

Status

MailNews Core
Backend
ASSIGNED
9 months ago
10 days ago

People

(Reporter: andrei030, Assigned: Jorg K (GMT+2), NeedInfo)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(5 attachments, 3 obsolete attachments)

(Reporter)

Description

9 months ago
Created attachment 8814239 [details]
Sin título.png

User Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:50.0) Gecko/20100101 Firefox/50.0
Build ID: 20161104212021

Steps to reproduce:

Nothing, clean installation.


Actual results:

Name for "Deleted" folder is displayed in English instead of Spanish language.
(Assignee)

Comment 1

9 months ago
¡Está mal! ;-) Which version are you using?
Assignee: nobody → rpmdisguise-nave
Component: Untriaged → es-ES / Spanish
Product: Thunderbird → Mozilla Localizations
(Assignee)

Comment 2

9 months ago
Actually, we need the version and the exact localisation, Spanish from Spain or Argentina, Chile, Mexico.
https://www.youtube.com/watch?v=eyGFz-zIjHE - ver a partir del minuto 2:12 ;-)
Yes, specify the regional variant, Thunderbird version and the exact steps you follow to reproduce the error. I've just used Transvision to look up the word and none of the four localizations seem to have that word untranslated.

Also, please note that the en-US word for the folder where deleted items are placed is not named "Deleted", but "Trash". I actually think you're seeing the name of a folder already created in an IMAP account.
(Reporter)

Comment 4

9 months ago
Sorry I didn't specified, the version is the last one, 45.5.0, and the localization es-ES.
Created attachment 8814481 [details]
es-ES name for Trash folder in Local Folders and an IMAP (Gmail) account

So, this is definitely due to your specific IMAP account. Both the local folders and a GMail account Trash folder are translated as "Papelera2.
(Assignee)

Comment 6

9 months ago
Thanks for clarifying.
Status: UNCONFIRMED → RESOLVED
Last Resolved: 9 months ago
Resolution: --- → INVALID
(Reporter)

Comment 7

9 months ago
So I have to deal with it and leave it in English, OK.
(Assignee)

Comment 8

9 months ago
Can't you rename the folder at IMAP level? Use the web interface of the provider to rename it? Or check the subscribe menu.
(Reporter)

Comment 9

9 months ago
It's a software problem, the folder cannot be renamed in any way. If I use the browser to enter my mail account the name is displayed correctly in Spanish. 

All the folders are named automatically and correctly by Thunderbird except the Deleted folder. For example:

Deleted | imap://*@imap-mail.outlook.com/Deleted | Deleted
Inbox | imap://*@imap-mail.outlook.com/INBOX | Bandeja de entrada
Junk | imap://*@imap-mail.outlook.com/Junk | Correo no deseado
Etc. 

If the folders are named by Thunderbird is to be expected that all of them are, and not all except the Deleted one.

I also tried to force rename using about:config, mail.server.server1.trash_folder_name, but it doesn't save my modifications, it keeps switching back to "Deleted" by itself.
(Assignee)

Comment 10

9 months ago
OK, I configured a Hotmail/Outlook/Live account in a Spanish version of TB 45.x and the folder is called "Deleted". In an English version of TB, the folder is also called "Deleted" not "Trash".

Let's look at it from the Thunderbird side again.
Summary: Name for Deleted folder not displayed correctly in Spanish language → Outlook/Hotmail trash folder is called "Deleted" in English and localised versions of Thunderbird
(Assignee)

Updated

9 months ago
Status: RESOLVED → REOPENED
Ever confirmed: true
Resolution: INVALID → ---
(Assignee)

Updated

9 months ago
Assignee: rpmdisguise-nave → nobody
Component: es-ES / Spanish → General
Product: Mozilla Localizations → Thunderbird
Version: unspecified → 45 Branch
(Assignee)

Comment 11

9 months ago
Kent, Magnus, how do the "special" folders get their names? Why would the Outlook/Trash folder be called "Deleted" in English and localised versions of TB?

I did an experiment on Outlook. I created a folder "Deleted" and got told that this created a name conflict. So obviously the Outlook folder is called "Deleted" internally, but since TB recognises it as special folder, why doesn't it assign a special name?
Flags: needinfo?(rkent)
Flags: needinfo?(mkmelin+mozilla)

Comment 12

9 months ago
It's a bit complex. RFC 6154 makes us set some flags. See nsMsgDBFolder::SetPrettyName and there around.
Flags: needinfo?(mkmelin+mozilla)
(Assignee)

Comment 13

9 months ago
Created attachment 8814729 [details] [diff] [review]
1320191-deleted-folder.patch

This works. With this patch, the folder is called "Trash" and would be localised properly.
Assignee: nobody → jorgk
Status: REOPENED → ASSIGNED
Flags: needinfo?(rkent)
Attachment #8814729 - Flags: review?(mkmelin+mozilla)

Comment 14

9 months ago
Comment on attachment 8814729 [details] [diff] [review]
1320191-deleted-folder.patch

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

I don't think this is right. The "name" should get set correctly to internally "trash" from somewhere... I don't know the details but it's complicated. We map up "our" special names to what the server tells us, if it tells us. Then localize them if necessary.
Attachment #8814729 - Flags: review?(mkmelin+mozilla) → review-
Component: General → Backend
Product: Thunderbird → MailNews Core
Version: 45 Branch → 45
(Reporter)

Comment 15

9 months ago
Thank you for your attention guys!
(Assignee)

Comment 16

9 months ago
This is indeed complicated.

Following http://kb.mozillazine.org/IMAP_Trash_folder I deleted the preference mail.server.server#.trash_folder_name

with the effect that I now have a Trash folder (which most likely TB will use for deleted items) and a Deleted folder (which most likely Hotmail will use). Most likely the Trash folder would be localised.

Reporter, if you're interested, delete/reset the preference mail.server.server#.trash_folder_name (you need to replace # with the real number) and report back what happens.

I still think there is a problem in reliably detecting the Hotmail Deleted folder. Maybe we need to tweak in nsImapIncomingServer::DiscoveryDone(). Well, on second thought, the folder *is* detected, but it's then not subject to localisation since it's only localised if it was originally called "Trash". Otherwise TB assumes that the name found is the name that the user wants to see.
(Assignee)

Comment 17

9 months ago
Summarising:
The problem seems to be that Micro$oft in their infinite wisdom don't use the standard trash name "Trash" (like all other IMAP services I know) but instead "Deleted". TB only localises if the folder is originally called "Trash" and hands all other names on to the user interface, most likely since it already assumes them to be localised.

So I'd have to see where to tweak this since Magnus didn't like the hack I presented. Looks like we'll have to treat M$ servers differently, just like we treat Gmail differently.

Comment 18

9 months ago
Treating differently probably isn't the thing to do. I'm not sure we actually do that for Gmail folders either, IIRC we're using what server tells us to.

As I recall, some things to be aware of
 - imap servers may tell you the special folders
 - we support imap servers having special folders localized server side (I don't recall what we display for those)
 - thunderbird will let you run localized thunderbird versions, but still with same folder names, so you can switch back and forth (for POP)
(Assignee)

Comment 19

9 months ago
Gmail is treated differently:
https://dxr.mozilla.org/comm-central/rev/4f8486123b817d6b18bf2684a0f7e5facf725ce6/mailnews/imap/src/nsImapIncomingServer.cpp#1547

> we support imap servers having special folders localized server side
Yes. But you can't distinguish between a localisation on the server side and "Deleted". Both are not the standard name "Trash".

> imap servers may tell you the special folders
Yes:
https://dxr.mozilla.org/comm-central/rev/4f8486123b817d6b18bf2684a0f7e5facf725ce6/mailnews/imap/src/nsImapIncomingServer.cpp#1582

The problem is that we only display the localised name if the original name was "Trash" which is not the case for M$. So in my patch I localise when "Trash" or "Deleted" is found. This is the only solution I can think of, unless we want to treat M$ differently. But I wouldn't know how. Gmail at least is recognisable by their extra bit:
https://dxr.mozilla.org/comm-central/rev/4f8486123b817d6b18bf2684a0f7e5facf725ce6/mailnews/imap/src/nsImapCore.h#149
I didn't see anything like this of M$.

So maybe you'd like to reconsider the r- here.
(Reporter)

Comment 20

9 months ago
I've tried to rename or restore "trash_folder_name" before, it always reset back to "Deleted". I've also tried to delete it completely editing prefs.js and editing it and make the file read only, doesn't work, it sets back to "Deleted" all the time.
Comment on attachment 8814729 [details] [diff] [review]
1320191-deleted-folder.patch

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

> -  else if (mFlags & nsMsgFolderFlags::Trash && name.LowerCaseEqualsLiteral("trash"))
> +  else if (mFlags & nsMsgFolderFlags::Trash &&    // Microsoft calls the folder "Deleted".
> +           (name.LowerCaseEqualsLiteral("trash") || name.LowerCaseEqualsLiteral("deleted")))

Oh, it's similar to filename =Unsent Messages, en-US folder name=Outbox, localized "Outbox" in each localized  build.
In this case,  "folder name=Outbox" is wriiten in Unsent Messages.msf, and, IIRC, "localized Outbox of  a localized  build" was written in .msf file in special situation.
I actually saw "related msgStore name=trash" in Trash.msf when "trash" was created by user, and I saw  "localized Trash of  a localized  build" in Trash.msf.
This was a cause of phenomenon of "Trash disappered, I can't delete mail!" which seems to still occur in some user's  non-ordinal environment.

If done on "Deleted", "Sent Items", "Sent Mail", "bin", "Draft"(Yahoo! IMAP)  and so on, I belive localized name for them should be provided, instead of "faked localized name via. faked folder name".
I think we are better distinguish following:
a) Use Deleted as special folder in Thunderbird automatically, as Trash is used as special folder in Thunderbird automatically.
b) Show localized name for Deleted if Deleted is used as "Special Folder of Trash" via. Server Settings.
To Jorg K.
I think method provided by your patch sample is partially correct for enhancement of b) in previous my comment.
  If folder has "Special Folder attribute of Trash"
  && if the folder's prettiestName.toLowerCase() is contained in table of TraseNameSet,
    true==TraseNameSet.has(prettiestName.toLowerCase() of the folder) where TraseNameSet=new Set(["trash","deleted","bin",...])
  Use it as-is in en-US build and use localized name for it in localized build at Folder Pane/Folder Picker.
  After it, all is up to localizer of localized build on localized name of "Deleted", "bin" etc.
  Then, localizer can freely create localized name for "Deleted", "deleted", "DeLeTed", ... .
(Assignee)

Comment 24

9 months ago
Magnus, putting NI for you to reconsider the patch or give some better instructions so we don't forget this.
Flags: needinfo?(mkmelin+mozilla)
(Assignee)

Comment 25

16 days ago
Comment on attachment 8814729 [details] [diff] [review]
1320191-deleted-folder.patch

Sorry Kent to trouble you with this review.

Maybe we can keep this brief. Use TB on a Hotmail account and observe that the trash folder is called "Deleted".

What happens?

I folder called "Deleted" identifies itself to TB as unique trash at
https://dxr.mozilla.org/comm-central/rev/4f8486123b817d6b18bf2684a0f7e5facf725ce6/mailnews/imap/src/nsImapIncomingServer.cpp#1582.

It sets the preference mail.server.server1.trash_folder_name to Deleted.

OK. So far so good. But what has really happened? TB recognised a trash folder with a non-standard name as trash, like if the IMAP server were saying: My trash is called (Spanish) "Papelera". So then TB will not change the trash name to the localised name here:
https://dxr.mozilla.org/comm-central/rev/f593d2d106c15ad9682a35eaa1a8679e7127acd6/mailnews/base/util/nsMsgDBFolder.cpp#3414
since it's already considered localised.

The only option to force Hotmail's Deleted folder to be localised is what I've proposed in the patch, to consider "Deleted" also as a standard name that will trigger localisation.

This bug has been hanging around for more than six months, so it's time to move it.
Flags: needinfo?(mkmelin+mozilla)
Attachment #8814729 - Flags: review?(rkent)

Comment 26

16 days ago
I have fought this issue quite a bit in ExQuilla (compounded by poor IDL character strings that do not properly encode non-ascii characters). What you have to really be careful of is creating duplicate folders.

The problems are, as is typical, the conflation of several concepts.

1) The folder URL comes directly from the file folder name, which must be the mail-expected canonical value (such as inbox, or trash). For all folders except for the canonical ones, the URL is the actual folder name. The canonical ones though expect particular values. "trash" for example. Sometimes if missing, they will get recreated, hence the duplicate folders.

2) Although folders like IMAP and EWS have the concept of a canonical "trash" folder, how is that indicated? You would think it would be using the folder flag, but the flag moves to a different, non-canonical folder when a user specifies a different folder for trash, junk, sent, etc. So at the point we are at here ins SetPrettyName, the folder URL (or name) is the correct way to indicate the canonical folder, not the flag. Checking for "deleted" seems strange to me.

So getting this correct is much harder than you think, because the underlying design is flawed so we are putting kludge on top of kludge.

The correct thing to do is to fix the underlying conflation problem. I am not saying we should not attempt a kludge, but this is why it is hard to review this apparently simple change.

For this particular case as I understand it, the question I would ask is why does a folder get created with the URL (name) "deleted"? Isn't imap recognizing it as canonical?
(Assignee)

Comment 27

16 days ago
Thanks for the detailed reply.

(In reply to Kent James (:rkent) from comment #26)
> For this particular case as I understand it, the question I would ask is why
> does a folder get created with the URL (name) "deleted"? Isn't imap
> recognizing it as canonical?
How would IMAP recognise it as canonical and where is that code?

I'm looking at 
https://dxr.mozilla.org/comm-central/rev/4f8486123b817d6b18bf2684a0f7e5facf725ce6/mailnews/imap/src/nsImapIncomingServer.cpp#1532

Let me copy it here and talk about it:

so first we get folders which carry the "trash" flag.

    rv = rootMsgFolder->GetFoldersWithFlags(nsMsgFolderFlags::Trash,
                                            getter_AddRefs(trashFolders));

    if (NS_SUCCEEDED(rv) && trashFolders)
    {
      uint32_t numFolders;
      trashFolders->GetLength(&numFolders);
      nsAutoString trashName;

here we get the trash folder name from the server specific preference.
We end up in nsImapIncomingServer::GetTrashFolderName()

      if (NS_SUCCEEDED(GetTrashFolderName(trashName)))
      {

loop over the folders and process each one

        for (uint32_t i = 0; i < numFolders; i++)
        {
          nsCOMPtr<nsIMsgFolder> trashFolder(do_QueryElementAt(trashFolders, i));
          if (trashFolder)
          {
            // If we're a gmail server, we clear the trash flags from folder(s)
            // without the kImapXListTrash flag. For normal servers, we clear
            // the trash folder flag if the folder name doesn't match the
            // pref trash folder name.
            if (isGMailServer)
            {
              nsCOMPtr<nsIMsgImapMailFolder> imapFolder(do_QueryInterface(trashFolder));
              int32_t boxFlags;
              imapFolder->GetBoxFlags(&boxFlags);
              if (boxFlags & kImapXListTrash)
              {
                continue;
              }
            }
            else
            {

we're not doing Gmail

              // trashName is the leaf name on the folder URI, which will be
              // different from the folder GetName if the trash name is
              // localized.
              nsAutoCString trashURL;
              trashFolder->GetFolderURL(trashURL);
              int32_t leafPos = trashURL.RFindChar('/');
              nsAutoCString unescapedName;

get the folder name from the URL.
              MsgUnescapeString(Substring(trashURL, leafPos + 1),
                                nsINetUtil::ESCAPE_URL_PATH, unescapedName);
              nsAutoString nameUnicode;

              if (NS_FAILED(CopyMUTF7toUTF16(unescapedName, nameUnicode)) ||
                  trashName.Equals(nameUnicode))
              {

If it equals what we already have in the preference, we're done.

                continue;
              }
              if (numFolders == 1)
              {
                // We got here because the preferred trash folder does not
                // exist, but a folder got discovered to be the trash folder.

We found a folder which is the unique trash, so store it's name in the preference and done.

                SetUnicharValue(PREF_TRASH_FOLDER_NAME, nameUnicode);
                continue;
              }
            }

If we get here, we'll just clear the "trash" flag.

            trashFolder->ClearFlag(nsMsgFolderFlags::Trash);
          }
        }
      }
    }
  }

So the code above will locate a unique folder with the trash flag, and store it's URL name in the preference.

That folder might be called "Trash", or "Deleted" or "Papelera". Trash will subject that to localisation, "Deleted" or "Papelera" will be assumed to be localised already and NOT trigger the code at:

https://dxr.mozilla.org/comm-central/rev/f593d2d106c15ad9682a35eaa1a8679e7127acd6/mailnews/base/util/nsMsgDBFolder.cpp#3414

So I'm not sure what you mean by "canonical" name or where that would be detected. 

The idea of the patch is to allow "Deleted" as well as "Trash" as folder names for the trash folder so that the localised name will be used.

For a Hotmail user of a Spanish TB this will happen: TB queries Hotmail and retrieves these special folders:
Inbox, Junk, Deleted, etc.

Inbox and Junk match TB's naming scheme and so the code around the patch uses kLocalizedInboxName, for Junk it uses kLocalizedJunkName. But "Deleted" doesn't match, so it's passed on as already localised. So the Spanish user sees a bunch of localised names, with a non-localised one staring at him in the middle.

So if there's something about canonical folders I'm missing, please let me know. If not, the patch must be correct.

Aceman said he knows something about this, so NI'ing him. If I read Wada's comment #22 and comment #23 correctly, he agrees with me.
Flags: needinfo?(rkent)
Flags: needinfo?(acelists)

Comment 28

11 days ago
What I mean by "canonical trash folder", after looking more at IMAP specs and our code, is a folder with the \TRASH attribute per rfc-6154. You can search for kImapXListTrash in our code. Both WADA and Magnus have previously mentioned this issue, but I don't see any evidence that you understand it.

So for this server case, what I would ask is, is this server giving rfc-6154 responses for the folder, and in particular does this Deleted folder get properly flagged as the \TRASH folder by the server? If so, then our problem is that we are not properly noting that.

Could we have an IMAP log for the server during startup?
Flags: needinfo?(rkent)
(Assignee)

Comment 29

11 days ago
Created attachment 8895005 [details]
imap.log - IMAP log when setting up Hotmail account for the first time

Here are the lines that refer to the "Deleted" folder:

2017-08-08 10:40:57.636000 UTC - [Unnamed thread 00000143C66B1000]: I/IMAP 00000143C32D1000:imap-mail.outlook.com:A:CreateNewLineFromSocket: * LIST (\HasNoChildren \Trash) "/" Deleted
2017-08-08 10:40:58.095000 UTC - [Unnamed thread 00000143C66B1000]: I/IMAP 00000143C32D1000:imap-mail.outlook.com:A:CreateNewLineFromSocket: * LSUB (\HasNoChildren \Trash) "/" Deleted

Clearly the folder called "Deleted" has the \Trash flag.

More debugging to come in the next comment.
(Assignee)

Updated

11 days ago
Attachment #8895005 - Attachment mime type: text/x-log → text/plain
(Assignee)

Comment 30

11 days ago
Created attachment 8895008 [details] [diff] [review]
trash-folder-debugging.patch - Debug patch

Here is a debug patch that shows that Hotmail's deleted folder is properly recognised as trash, it has the kImapXListTrash flag and in turn we set nsMsgFolderFlags::Trash on the folder.

The debug I see is:
=== We recognised Deleted as the official trash folder!!
(Assignee)

Comment 31

11 days ago
Created attachment 8895011 [details] [diff] [review]
1320191-deleted-folder.patch (v1, rebased)

So here we have the very first patch again, but I needed to rebase it.

So let's look at the facts one more time:

Hotmail has a trash folder called "Deleted".
Hotmail communicates that fact by giving us \Trash on that folder.
We detect that folder as the trash folder.
So far, so good?

All the code I quoted in comment #27 works fine, too. There is a unique trash folder and we store its name into mail.server.server1.trash_folder_name.
I can see that being set to "Deleted".

The problem is in the code I'm proposing to change, let's look at it again:
-  else if (mFlags & nsMsgFolderFlags::Trash && name.LowerCaseEqualsLiteral("trash"))
+  else if (mFlags & nsMsgFolderFlags::Trash && hasTrashName(name))
     rv = SetName(kLocalizedTrashName);

If the folder has the trash flag, which our folder has, and it's called "Trash", when we use the localised name. OK, so if I have a "normal" IMAP server whose trash folder is called Trash, then my Spanish user will see "Papelera" (paper basket) instead of Trash.

With Hotmail's abnormal IMAP server, the localised name is not used, since "Trash" is not matched. So we en_US users see "Deleted" as the trash name, and our Spanish friends also see "Deleted" in between the other localised folders whose names matched.

So, if we want "Deleted" to get localised, we need to admit it next to "Trash" in the code I'm trying to change.

Or said differently: Localised names are only used, if the folders match Inbox, Junk, Drafts, Sent, Trash, etc. As soon as a different name is used, like "Deleted", Thunderbird assumes that the name is already localised and doesn't touch it further. We need to break that.
Attachment #8895011 - Flags: review?(rkent)
(Assignee)

Comment 32

11 days ago
I'll put the NI on again, since this has been sitting with r? for a while now.
Flags: needinfo?(rkent)

Comment 33

11 days ago
Thanks for debugging, but I think it's still wrong. 

I mean, if you know it has the trash flag you don't need to check the name (which you would need to to also for any other localized server folder names). But I'm not sure that's not good either, since you may want to know what folder is the trash folder. Also, I know I've had some server that had something like Deleted and also Trash, and what would happen if you also have the folder *named* (localized) Trash? That could look very strange.
(Assignee)

Comment 34

11 days ago
(In reply to Magnus Melin from comment #33)
> Thanks for debugging, but I think it's still wrong.
So, if it's still wrong, then what is right?

> I mean, if you know it has the trash flag you don't need to check the name
> (which you would need to to also for any other localized server folder
> names).
Well, looking at:
  else if (mFlags & nsMsgFolderFlags::Trash && name.LowerCaseEqualsLiteral("trash"))
    rv = SetName(kLocalizedTrashName);
are you saying to make it just:
  else if (mFlags & nsMsgFolderFlags::Trash)
    rv = SetName(kLocalizedTrashName);
?
The result would be that if a Spanish mail provider had "Basura" (rubbish) with \Trash, then the Spanish user would see "Papelera" since we'd always localise.

I know that the patch is on thin ice, we just add Deleted since Hotmail is using it, but as WADA pointed out (comment #23), some crazy server could call the trash "Bin", and we'd then show that unlocalised. This patch is a compromise to get localised Hotmail users a decent UI without doing damage elsewhere.

> But I'm not sure that's not good either, since you may want to know
> what folder is the trash folder.
I don't follow that. "you may want to know what folder is the trash folder". The one with the special icon, typically also recognisable by it's name. I agree that needlessly forcing localised names is not such a good idea, so once again, here I'm proposing a compromise.

Note the following: The localised name for "Trash" in en-GB is "Deleted", so we're not upsetting any Brits by showing them "Trash" where they saw deleted before. Only en-US Hotmail users will see "Trash" whereas on the server they see "Deleted".

Another option would be to only force the translation of "Deleted" is the app locale isn't English ;-)

> Also, I know I've had some server that had
> something like Deleted and also Trash, and what would happen if you also
> have the folder *named* (localized) Trash? That could look very strange.
I don't follow that. If the server has two \Trash folders, when we both localise them. But that shouldn't happen.
If you have
  Trash \Trash
  Deleted (no flag)
you get (assuming Spanish): Papelera and Deleted.

Or if a Hotmail user creates a Trash folder,
  Trash (no flag)
  Deletes \Trash
that becomes Trash and Papelera. Where is the problem?

Comment 35

11 days ago
(In reply to Jorg K (GMT+2) from comment #34)
> are you saying to make it just:
>   else if (mFlags & nsMsgFolderFlags::Trash)
>     rv = SetName(kLocalizedTrashName);
> ?

It's an alternative, I'd say preferable to hardcoding a check for the name Deleted. 

But you would potentially end up with two "Trash" folders.

> > But I'm not sure that's not good either, since you may want to know
> > what folder is the trash folder.
> I don't follow that. "you may want to know what folder is the trash folder".
> The one with the special icon, typically also recognisable by it's name. I

What I mean is the folder have a name on the folder and you may want to know what that is, especially when talking to other people who maybe aren't using Thunderbird.

> agree that needlessly forcing localised names is not such a good idea, so
> once again, here I'm proposing a compromise.
> 
> Note the following: The localised name for "Trash" in en-GB is "Deleted", so
> we're not upsetting any Brits by showing them "Trash" where they saw deleted
> before. Only en-US Hotmail users will see "Trash" whereas on the server they
> see "Deleted".
> 
> Another option would be to only force the translation of "Deleted" is the
> app locale isn't English ;-)
> 
> > Also, I know I've had some server that had
> > something like Deleted and also Trash, and what would happen if you also
> > have the folder *named* (localized) Trash? That could look very strange.
> I don't follow that. If the server has two \Trash folders, when we both
> localise them. But that shouldn't happen.

Only one folder has the \Trash flag. What I mean is you could have a normal folder Trash, and then the Deleted folder with the \Trash flag, Then with your patch you end up showing two Trash folders in the UI.

> If you have
>   Trash \Trash
>   Deleted (no flag)
> you get (assuming Spanish): Papelera and Deleted.
> 
> Or if a Hotmail user creates a Trash folder,
>   Trash (no flag)
>   Deletes \Trash
> that becomes Trash and Papelera. Where is the problem?

You're assuming the translation is not English. For en-US you get Trash and Trash.
(Assignee)

Comment 36

11 days ago
OK, I'll do a patch where we check the locale.
(Assignee)

Comment 37

10 days ago
Created attachment 8895094 [details] [diff] [review]
1320191-deleted-folder.patch (v2)

OK, here with a locale check. So in the unlikely case that the user has a Trash folder and a Deleted \Trash folder, I don't turn the Deleted folder into Trash any more.

So in an English app, the Deleted folder is not shown as Trash, so all the US Hotmail users see what they are used to. For non-English users we assume that we need to use the localised name for Trash and Deleted if they have the trash flag.

Zibi, can you please check the code and tell me whether there's a better way to check for non-English. I wanted to avoid Substr, etc. since most of that doesn't work on nsCString, you need an nsAutoCString for that.
Attachment #8895011 - Attachment is obsolete: true
Attachment #8895011 - Flags: review?(rkent)
Flags: needinfo?(acelists)
Attachment #8895094 - Flags: review?(rkent)
Attachment #8895094 - Flags: feedback?(mkmelin+mozilla)
Attachment #8895094 - Flags: feedback?(gandalf)
Comment on attachment 8895094 [details] [diff] [review]
1320191-deleted-folder.patch (v2)

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

hah, I don't know if there's a simpler way, but there's definitely a better way :)

You can get use `LocaleService::GetAppLocaleAsLangTag` to just get the first from the list (since you don't care about the fallback).

Then:

```
static bool
nonEnglishApp()
{
  nsAutoCString locale;
  LocaleService::GetInstance()->GetAppLocaleAsLangTag(locale);
  
  Locale loc = LocaleService::Locale(locale, true); // take a range
  Locale enUSLoc = LocaleService::Locale("en", true); // take a range

  return loc.LanguageMatches(enUSLoc);
}
```

This may look like more code, but I don't think it actually is that much more, while it definitely is way more bullet proof.
Attachment #8895094 - Flags: feedback?(gandalf)
(Assignee)

Comment 39

10 days ago
Thanks, but I can't get this to compile. 'Locale' seems to be private.

LocaleService.h:
private:
  /**
   * Locale object, a BCP47-style tag decomposed into subtags for
   * matching purposes.
   *
   * If constructed with aRange = true, any missing subtags will be
   * set to "*".
   */
  class Locale
(Assignee)

Comment 40

10 days ago
Created attachment 8895116 [details] [diff] [review]
1320191-deleted-folder.patch (v2b)

OK, slightly improved locale handling, still now that Zibi suggested, but that doesn't compile, well, I wrestled with it for a while.

'Locale' doesn't appear to be available and LocaleService::Locale() seems to be called 'LocaleService::Locale::Locale':
https://dxr.mozilla.org/comm-central/rev/47248637eafa9a38dade8dc3aa6c4736177c8d8d/mozilla/intl/locale/LocaleService.cpp#725

Anyway, Kent and Magnus need to decide whether they like this approach or not, regardless of the most elegant way to check for English.
Attachment #8895094 - Attachment is obsolete: true
Attachment #8895094 - Flags: review?(rkent)
Attachment #8895094 - Flags: feedback?(mkmelin+mozilla)
Attachment #8895116 - Flags: review?(rkent)
Attachment #8895116 - Flags: feedback?(mkmelin+mozilla)
(Assignee)

Comment 41

10 days ago
Typo: still NOT that Zibi suggested. Zibi, what do you say?
Flags: needinfo?(gandalf)
Ooh, sorry! Yeah. We'll need bug 1349377 first :(

The new patch looks ok, except that I'd change it to 0-2 and "en" to match "en" as well :)

return !Substring(locale, 0, 3).EqualsLiteral("en-");
Flags: needinfo?(gandalf)
(Assignee)

Comment 43

10 days ago
OK, I'll do that after the review to avoid more noise. Thanks again.
(Assignee)

Updated

10 days ago
Attachment #8814729 - Attachment is obsolete: true
Attachment #8814729 - Flags: review?(rkent)
You need to log in before you can comment on or make changes to this bug.