Closed Bug 1178197 Opened 7 years ago Closed 6 years ago

Sent folder confusion issues

Categories

(Firefox OS Graveyard :: Gaia::E-Mail, defect)

ARM
Gonk (Firefox OS)
defect
Not set
blocker

Tracking

(b2g-v2.1 wontfix, b2g-v2.2 affected, b2g-v2.5 fixed, b2g-master fixed)

RESOLVED FIXED
Tracking Status
b2g-v2.1 --- wontfix
b2g-v2.2 --- affected
b2g-v2.5 --- fixed
b2g-master --- fixed

People

(Reporter: gerard-majax, Assigned: asuth)

References

Details

(Keywords: foxfood, regression)

Attachments

(3 files)

Attached file logcat while sending
This has been broken for a while now and it is still present on current master, but I have checked everything and can only reproduce on my Fastmail-hosted account. Mozilla Email do not expose this issue. I have even destroyed and re-created my account in Gaia Email client, unsuccessfully.

STR:
 0. Send email from Fastmail-hosted account
 1. Wait the toaster "Email sent"
 2. Go to "Sent Items" folder

Expected:
 I see my email

Actual:
 I don't see it
Flags: needinfo?(bugmail)
The in-app debugging fails to dump the circular log buffer:
> 06-29 10:32:14.252  2784  2784 E E-Mail  : [JavaScript Error: "DataCloneError: The object could not be cloned." {file: "app://email.gaiamobile.org/js/ext/main-frame-setup.js" line: 3830}]
It looks like this is the ID error we speculated about a bit before on IRC, but with this specific logcat (thank you!) what is happening is much more obvious.  The biggest problem here is that we're letting the ID command failure cascade in such a way that it breaks other things.  The lesser problem is that browserbox is being over-eager about sending the ID command (from my memory), although one could also argue that the transparent proxies should be more understanding of these things in the interest of latency.

This is likely to be fairly fastmail specific because of their use of a modified nginx proxy; dovecot-based proxies are unlikely to have this problem and the other proxies in use out there don't seem to do this either.
Flags: needinfo?(bugmail)
Summary: Sent emails are not copied to the "Sent Items" folder on Fastmail account → Sent emails are not copied to the "Sent Items" folder on Fastmail account because of error handling cascade when we blindly send ID that nginx does not care about/understand
QA Whiteboard: [foxfood-triage]
Any news?
Flags: needinfo?(bugmail)
The ID error turned out to be harmless and misleading.  I did file a support request with fastmail to ask them to not advertise "ID" in their "CAPABILITY" list for their nginx proxy if they're just going to return "BAD" in the interest of avoiding that waste, however.

This is a folder mismatch issue.  At least for me, on my fastmail account which I migrated from a Dovecot server where I used Thunderbird (and a Sent folder existed because either Thunderbird put it there or it already existed), I ended up with what amounts to 2 sent folders (as visible in a `tag LIST "" "*"` request result):
* LIST (\HasNoChildren) "." Sent
* LIST (\HasNoChildren \Sent) "." "Sent Items"

FxOS email is putting the message in "Sent" (as is Thunderbird) because our folder-type heuristics identify it as a a sent folder as well.  And our sent folder logic just picks the first one because we've got to fix one.  Fast Mail's web UI by default uses the "Sent Items" folder.  (And oddly, they localize it as "Sent" for me up at the top of the web UI's folder tree since it has the magic special-use flag, and then the actual "Sent" folder is also displayed as "Sent" down at the bottom of the tree since that's alphabetically where it goes.


I expect your situation is similar, although your duplicate sent folder may be named "envoyés" and we'll detect it.  If you could please check for duplicate sent folders that would be helpful.

Assuming that's your problem too, the only real fix is to consolidate the folders into one and make sure all your mail clients know to use that folder and not re-create the one you delete.  Note that Thunderbird probably needs to be explicitly told to use a different folder, deleting the folder is just going to make Thunderbird create it again.  FxOS email will figure things out once it updates its folder list, but it does that at most every 24 hours on opening the app, so the most pragmatic solution if you don't mind the extra data usage and want the app happy immediately is to remove and re-create your account.
Flags: needinfo?(bugmail) → needinfo?(lissyx+mozillians)
Summary: Sent emails are not copied to the "Sent Items" folder on Fastmail account because of error handling cascade when we blindly send ID that nginx does not care about/understand → Sent folder confusion issues (likely on fastmail if the account migration feature is used?)
I did use the migration from my own courier-imap hosted at home server, and I do have two folders: "Sent" and "Sent Items".
Flags: needinfo?(lissyx+mozillians)
"Sent" was empty, all my thunderbird instances were configured to make use of "Sent Items". So I deleted it, I'll see how it goes ...
So, those messages should have ended up somewhere, though.  If the Gaia email messages weren't in "Sent" and they're not in "Sent Items", then we still have a mystery.  One relevant factor might be whether you're using port 993 for IMAP that uses the "INBOX." namespace or port 992 (autoconfig will pick this) that ditches that namespace and puts things at the root.  I tested using port 992; maybe we do something weird and dumb on port 993 and end up using INBOX.Inbox.Sent or something?
They weren't in Sent for sure. And It's not configured with autoconfig, and it makes use of port 993.
So they were NOT even copied to the Sent folder, and on a device where the folder list has been updated since (I only see "Sent Items"), this is still not properly copied ...
Flags: needinfo?(bugmail)
My inclination would still be to suspect that they're going somewhere since the job does seem to complete successfully.

As a quick test, could you compose and send an email (not to yourself) and do something like make the subject be "emailgoneemailgone", send it, and then use the fastmail web UI to search for that string and see if it finds the message?

Note that there is a workaround of sorts, which is that on the per-identity settings in fastmail, there's a box you can check to make the act of sending a message via SMTP automatically put the message in the sent folder.
Flags: needinfo?(bugmail) → needinfo?(lissyx+mozillians)
Er, and by "not to yourself", I just mean, we don't want that specific account receiving a copy of the mail.
Nice trick. It has been indeed lurred into another subdirectory which is called "Sent". But the whole path is "INBOX.IDN.Sent" :(
Flags: needinfo?(lissyx+mozillians) → needinfo?(bugmail)
Whoa.  That's a weird path.  But, ugh, I see that in bug 854128 we already knew about INBOX.IDN.Sent; I need to remember to check your historical bugs! ;)  The good news about this is at least that it seems like we probably didn't create that folder.  But we/I must have regressed your fix in bug 854128 at some point.
Although I suppose it's also worth checking if the folder somehow got a special-use flag stuck on it somehow.  A `tag LIST "" "*"` run against your IMAP server would illustrate if that's happening.
(In reply to Andrew Sutherland [:asuth] from comment #14)
> Although I suppose it's also worth checking if the folder somehow got a
> special-use flag stuck on it somehow.  A `tag LIST "" "*"` run against your
> IMAP server would illustrate if that's happening.

So, as said on IRC, regression :)
Blocks: 854128
Keywords: regression
Summary: Sent folder confusion issues (likely on fastmail if the account migration feature is used?) → Sent folder confusion issues
Investigation revealed that the problem is BrowserBox's own name-inference logic not being path-aware.  And the test_account_folder_logic.js was a bit too much of a unit test since it didn't involve BrowserBox.  I added a test case to test_imap_sync_folder_list.js which is end-to-end and it duplicated the problem.  Unfortunately there's also a minor test infrastructure issue there because of the test file wanting to create a duplicate account, so that needs to be dealt with.

But to address the functional problem, I filed https://github.com/whiteout-io/browserbox/issues/67 against browserbox.  It shouldn't take long for :andris9 to respond with how he's interested in us addressing the problem, and then I can provide a patch and fix up the tests here too.
Assignee: nobody → bugmail
Status: NEW → ASSIGNED
Flags: needinfo?(bugmail)
Any news ?
Flags: needinfo?(bugmail)
Flags: needinfo?(bugmail)
Are we going to ship 2.5 with that bug?
Flags: needinfo?(bugmail)
[Blocking Requested - why for this release]:
blocking-b2g: --- → 2.5?
Comment on attachment 8682902 [details] [review]
[gaia-email-libs-and-more] asutherland:sent-name-folder-issues > mozilla-b2g:master

The fix itself is pretty simple; we're taking the following two commits which make browserbox stop forcing its inference logic on us:
https://github.com/whiteout-io/browserbox/commit/ba1fd503ffb6fbd3438eba8ce08b0192e5a457ef
https://github.com/whiteout-io/browserbox/commit/60f5ba87c30485364a80467224669ca975f25919

We're not taking the most recent upstream version because:
- One has not been cut.
- It includes other stuff not appropriate for uplift for risk reasons.

It also includes us taking an updated mail-fakeservers npm release that I've speculatively released:
https://github.com/mozilla-b2g/mail-fakeservers/commit/43eb6b32c671d01accc79595fe01abc187955a5a

The problem there was that hoodiecrow (the fake IMAP server) and our hookup to it was not resulting in gelam being told the right NAMESPACE, so our heuristics couldn't work meaning my cool new test case failed.
Flags: needinfo?(bugmail)
Attachment #8682902 - Flags: review?(jrburke)
Uh, and here was the hoodiecrow issue I had raised that's not marked close yet I guess because there's no release cut: https://github.com/whiteout-io/browserbox/issues/67
Attachment #8682902 - Flags: review?(jrburke) → review+
Is 2.5+ at this point basically an uplift request? Is it OK to upgrade our browserbox version at this point in the 2.5 cycle?
Flags: needinfo?(bugmail)
I do think this is fine and safe to uplift.  My comment 21 may not have been sufficiently clear: we are *not* taking a new version of browserbox.  We are taking a +1/-1 patch that explicitly only changes logic related to folder metadata and for which we already had unit test coverage and now have even better unit test coverage.

I personally have not been of the mind that this bug actually constitutes a blocker, but I'm assuming Gregor didn't just randomly set the blocking flag, and we've got the fix and so I do think we should uplift it.  It can be just as an approval flag rather than a blocking flag.
Flags: needinfo?(bugmail)
Mahe, how are we handling 2.5 uplifts?
Flags: needinfo?(mpotharaju)
asuth, can you request approval‑gaia‑v2.5 on the patch and fill out the resulting questionnaire?
Flags: needinfo?(mpotharaju) → needinfo?(bugmail)
landed on trunk:
https://github.com/mozilla-b2g/gaia/pull/33002
https://github.com/mozilla-b2g/gaia/commit/e68d693cb55fb5d8946498eb2bdb63f55116d38e

https://github.com/mozilla-b2g/gaia-email-libs-and-more/pull/398
https://github.com/mozilla-b2g/gaia-email-libs-and-more/commit/5f99112998af5027d83325e1fde0e4e9e4d3c3b6

Note that there does appear to be a deterministic Gij22 failure that is unrelated to this change on the basis of it also being broken on trunk.
Status: ASSIGNED → RESOLVED
blocking-b2g: 2.5? → ---
Closed: 6 years ago
Flags: needinfo?(bugmail)
Resolution: --- → FIXED
Comment on attachment 8683240 [details] [review]
[gaia] asutherland:email-sent-folder > mozilla-b2g:master

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #):
This showed up in v2.1 with bug 885110.

[User impact] if declined:
Users with a folder named "Sent" (or equivalent under other languages) that is not top-level may be misidentified as the appropriate Sent folder for us to save sent messages into.  Specifically, this will occur when all of the following is true:
- It's an IMAP account on non-Gmail, non-Coremail servers
- The full path of the folder sorts earlier than the actual sent folder.

This is most likely to occur for long-term IMAP users who have crufty folder setups due to multiple account migrations or having used a wide variety of client and server mechanisms that created folders with abandon.

[Testing completed]:
Automated back-end test coverage.

[Risk to taking this patch]:
Not risky.  Very small, targeted change.  All risk mitigation possible has been mitigated.

[String changes made]:
No string changes.
Attachment #8683240 - Flags: review+
Attachment #8683240 - Flags: approval-gaia-v2.5?
Thanks Dylan for answering while was out. 

Will approve this patch.
Comment on attachment 8683240 [details] [review]
[gaia] asutherland:email-sent-folder > mozilla-b2g:master

Approved to land on 2.5

Thanks
Attachment #8683240 - Flags: approval-gaia-v2.5? → approval-gaia-v2.5+
You need to log in before you can comment on or make changes to this bug.