835149 - [email/IMAP] folder list sync breaks if IMAP server (ex: courier) returns folders in non-hierarchy order

Status: RESOLVED FIXED

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

Description

[Andrew Sutherland [:asuth] (he/him)]

:gerard-majax_ reported a problem with folder sync on his Courer IMAP on debian stable setup and provided great assistance in analyzing the problem and trying out a quick fix.

The patch I cooked up seemed to work and makes sense:
====
diff --git a/data/lib/imap.js b/data/lib/imap.js
index 9333237..ebf8a68 100644
--- a/data/lib/imap.js
+++ b/data/lib/imap.js
@@ -561,8 +561,9 @@ ImapConnection.prototype.connect = function(loginCb) {
               box.parent = parent;
             }
             box.displayName = decodeModifiedUtf7(name);
-            if (!curChildren[name])
-              curChildren[name] = box;
+            if (curChildren[name])
+              box.children = curChildren[name].children;
+            curChildren[name] = box;
           }
         break;
         // QRESYNC (when successful) generates a "VANISHED (EARLIER) uids"
====

It appears upstream node-imap shares this problem as well, so an effort should be made to provide an upstream fix.

Unit test-wise, this would be a great time to port the Thunderbird IMAP fake-server, although this is still simple enough that fawlty socket can probably fake it without too much trouble.


In terms of expected occurrences, we haven't heard of this one before and it seems neither has node-imap; I suspect Courier may be somewhat alone in its LIST ordering choices, although it wouldn't surprise me if other older/rarer servers like UW-IMAP might get up to this too.

Comment 1

[:gerard-majax]

This is an old setup of courier-imap, installed long time ago but still running fine. FYI, courier-imap is 4.8.0-3 in debian. I can confirm that with this small fix, I now have a fully working email client in Gaia :)

Comment 2

[Andrew Sutherland [:asuth] (he/him)]

Attachment: https://github.com/mozilla-b2g/gaia-email-libs-and-more/pull/117

r=squib on the gelam PR at https://github.com/mozilla-b2g/gaia-email-libs-and-more/pull/117

landed on gaia-email-libs-and-more/master:
https://github.com/mozilla-b2g/gaia-email-libs-and-more/pull/117

Comment 3

[Andrew Sutherland [:asuth] (he/him)]

Attachment: https://github.com/mozilla-b2g/gaia/pull/7857

propagated GELAM patch with r=squib to gaia/master.
landed on gaia/master:
https://github.com/mozilla-b2g/gaia/pull/7857

NOTE: If blocking-basecamp+ is set, just land it for now.

[Approval Request Comment]
User impact if declined: People using Courier IMAP may be unable to synchronize their folder list, resulting in only their Inboxes being available.  Secondary fallout relating to serious problems sending messages are expected in such a case.
Testing completed: Bug reporter tested the patch on a Courier IMAP system and it fixed it for them.  Unit tests for non-Courier IMAP continue to pass, manual testing against real IMAP servers.
Risk to taking this patch (and alternatives if risky): Super low risk.  Trying to avoid letting v1-train diverge from v2 too much, which is considered potentially troublesome.

Comment 4

[Andrew Sutherland [:asuth] (he/him)]

Comment on attachment 707981 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/7857

(see previous comment; accidentally left approval field blank)

Comment 5

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 707981 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/7857

Approved for the v1 train (i.e. the v1.0.1 release)

Comment 6

[Andrew Sutherland [:asuth] (he/him)]

(In reply to Jonas Sicking (:sicking) from comment #5)
> Approved for the v1 train (i.e. the v1.0.1 release)

landed on gaia/v1-train:
https://github.com/mozilla-b2g/gaia/pull/7871

Comment 8

[Lukas Blakk [:lsblakk] use ?needinfo]

Batch edit: bugs fixed on b2g18 since 1/25 branch of v1.0 are fixed on v1.0.1