Last Comment Bug 495318 - add support for LIST (SUBSCRIBED) part of IMAP LIST-EXTENDED command (RFC 5258)
: add support for LIST (SUBSCRIBED) part of IMAP LIST-EXTENDED command (RFC 5258)
Status: RESOLVED FIXED
:
Product: MailNews Core
Classification: Components
Component: Networking: IMAP (show other bugs)
: Trunk
: All All
: -- normal with 3 votes (vote)
: Thunderbird 19.0
Assigned To: David Lechner (:dlech)
:
:
Mentors:
http://tools.ietf.org/html/rfc5258
Depends on: 885162 816028
Blocks: 781443 RFC5258 928654 RFC6154 799821
  Show dependency treegraph
 
Reported: 2009-05-28 15:19 PDT by David :Bienvenu
Modified: 2015-06-22 08:18 PDT (History)
14 users (show)
mozilla: wanted‑thunderbird3+
ryanvm: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
fixed


Attachments
check for LIST-EXTENDED capibility when getting subscribed IMAP folders (4.21 KB, patch)
2012-09-24 09:50 PDT, David Lechner (:dlech)
no flags Details | Diff | Splinter Review
check for LIST-EXTENDED capibility when getting subscribed IMAP folders (7.52 KB, patch)
2012-09-24 11:09 PDT, David Lechner (:dlech)
mozilla: review+
Details | Diff | Splinter Review
related tests, etc. (66.48 KB, patch)
2012-10-08 22:54 PDT, David Lechner (:dlech)
no flags Details | Diff | Splinter Review
related tests, etc. v2 (66.57 KB, patch)
2012-10-09 08:38 PDT, David Lechner (:dlech)
irving: review+
Details | Diff | Splinter Review
related tests, etc. v3 (66.61 KB, patch)
2012-10-30 19:20 PDT, David Lechner (:dlech)
no flags Details | Diff | Splinter Review
fix v2 (66.63 KB, patch)
2012-10-31 10:54 PDT, David Lechner (:dlech)
no flags Details | Diff | Splinter Review
fix v2 (9.49 KB, patch)
2012-11-08 17:03 PST, David Lechner (:dlech)
irving: review+
Details | Diff | Splinter Review
related tests, etc. v4 (66.63 KB, patch)
2012-11-14 10:29 PST, David Lechner (:dlech)
no flags Details | Diff | Splinter Review
fix v3 (60.39 KB, patch)
2012-11-14 10:30 PST, David Lechner (:dlech)
no flags Details | Diff | Splinter Review
attempt to fix try server fails (9.96 KB, patch)
2012-11-15 22:27 PST, David Lechner (:dlech)
irving: review+
Details | Diff | Splinter Review
related tests, etc. v5 (72.63 KB, patch)
2012-11-19 14:09 PST, David Lechner (:dlech)
standard8: approval‑comm‑aurora+
Details | Diff | Splinter Review

Description David :Bienvenu 2009-05-28 15:19:21 PDT
Dovecot supports this (not sure who else does). It has the nice property that LIST-EXTENDED SUBSCRIBED gives you all the useful flags about a folder, like LIST does, but unlike LSUB. In particular, this will tell us things like /NoInferiors, which means we shouldn't even try to create sub-folders, which is important for Archive. I'm going to tentatively mark it blocking for that reason.
Comment 1 WADA 2009-05-29 03:43:38 PDT
DOCs for LIST-EXTENDED. Still Internet-Draft?
> http://www.iana.org/assignments/imap4-list-extended
> http://tools.ietf.org/html/draft-melnikov-imapext-status-in-list-00
Cyrus, Zimbra etc. also looks to support LIST-EXTENDED. 
> http://www.melnikov.ca/mel/devel/ServerReference.html
Comment 2 Nikolay Shopik 2009-05-29 05:10:50 PDT
Also we have bug 439048 for http://tools.ietf.org/html/draft-melnikov-imapext-status-in-list
Comment 3 David :Bienvenu 2009-07-08 17:10:49 PDT
I've fixed the Archive bug a different way, and the server it was happening on didn't support LIST-EXTENDED anyway, so this isn't a blocker. But it would still be very nice to have.
Comment 4 David Lechner (:dlech) 2012-09-24 09:50:19 PDT
Created attachment 664099 [details] [diff] [review]
check for LIST-EXTENDED capibility when getting subscribed IMAP folders

This is not full LIST-EXTENDED support, just LIST (SUBSCRIBED) as mentioned in the first comment. And I'll come up with a test soon.
Comment 5 Ben Bucksch (:BenB) 2012-09-24 09:53:59 PDT
-      else if (token.Equals("EXTENDED-LIST", nsCaseInsensitiveCStringComparator()))
+      else if (token.Equals("LIST-EXTENDED", nsCaseInsensitiveCStringComparator()))

Outch! I've probably looked at this a dozen times and never noticed it.
Comment 6 David Lechner (:dlech) 2012-09-24 11:09:58 PDT
Created attachment 664130 [details] [diff] [review]
check for LIST-EXTENDED capibility when getting subscribed IMAP folders

Added some flags and flag parsing based on RFC document.
Comment 7 Ben Bucksch (:BenB) 2012-09-25 02:07:39 PDT
Compare bug 558659 comment 40, TODO Step 3
Comment 8 David :Bienvenu 2012-09-30 15:40:00 PDT
Comment on attachment 664130 [details] [diff] [review]
check for LIST-EXTENDED capibility when getting subscribed IMAP folders

 thx, David. I don't have a server to test this with, but it seems OK.
Comment 9 David Lechner (:dlech) 2012-10-08 22:54:12 PDT
Created attachment 669415 [details] [diff] [review]
related tests, etc.

I have modified imapd.js to partially implement LIST-EXTENDED. I only implemented what was necessary to test this bug, but it should be very simple to complete the implementation if we need to.

In doing so, I also had to modify some of the GMail stuff. I have re-written the XLIST handler so that it works just like the official SPECIAL-USE imap extension. Again, implementing the rest of SPECIAL-USE should be rather trivial now.

I also threw in the CHILDREN imap extension for good measure (GMail uses it too).

I added a couple of tests in mailnews/base/test that test imapd.js to make sure that it spits out the proper responses when it is simulating LIST-EXTENDED and GMail servers.

Then finally, the test in mailnews/imap/test it the test to make sure the changes I made in actual mailnews code works properly.

I have run all tests in both mailnews/base/test and mailnews/imap/test to make sure I did not break anything. My GMail changes broke the couple of tests that atuljangra recently contributed, so I fixed them up.
Comment 10 David Lechner (:dlech) 2012-10-09 08:38:54 PDT
Created attachment 669557 [details] [diff] [review]
related tests, etc. v2

added an error check
Comment 11 :Irving Reid (No longer working on Firefox) 2012-10-30 11:46:18 PDT
Comment on attachment 669557 [details] [diff] [review]
related tests, etc. v2

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

I feel a bit guilty about the nits because the code is great.

::: mailnews/base/test/unit/test_testsuite_fakeserver_imapd_gmail.js
@@ +1,5 @@
> +/* This Source Code Form is subject to the terms of the Mozilla Public
> + * License, v. 2.0. If a copy of the MPL was not distributed with this
> + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
> +
> +// Test that imapd.js fakeserver correlecty emulates GMail server

correctly

@@ +31,5 @@
> +
> +// mbox mailboxes cannot contain both child mailboxes and messages, so this will
> +// be one test case.
> +function setupMailboxes()
> +{  

White space at end of line

@@ +43,5 @@
> +  gIMAPDaemon.createMailbox("[Gmail]/Trash", {specialUseFlag : "\\Trash"});
> +  gIMAPDaemon.createMailbox("test", {});
> +
> +  handler = gIMAPServer._readers[0]._handler;
> +  

White space

@@ +44,5 @@
> +  gIMAPDaemon.createMailbox("test", {});
> +
> +  handler = gIMAPServer._readers[0]._handler;
> +  
> +  // wait for imap pump to do it's thing or else we get memory leaks

its

Man, I'm picking all the nits today. Must be the rainy grey skies.

::: mailnews/base/test/unit/test_testsuite_fakeserver_imapd_list-extended.js
@@ +55,5 @@
> +
> +  // wait for imap pump to do it's thing or else we get memory leaks
> +  gIMAPInbox.updateFolderWithListener(null, asyncUrlListener);
> +  yield false;
> +}

Would it be worth the effort to extract the common setup and related code from all these tests? I'm not going to r- for it.

::: mailnews/imap/test/unit/test_gmailOfflineMsgStore.js
@@ +56,5 @@
> +  setup,
> +  updateFolder,
> +  selectInboxMsg,
> +  StreamMessageInbox,
> +  createAndUpdate,  

trailing whitespace

@@ +61,5 @@
> +  addFoo,
> +  updateFoo,
> +  selectFooMsg,
> +  StreamMessageFoo,
> +  crossStreaming,  

whitespace

@@ +75,5 @@
> +  setupIMAPPump("GMail");
> +
> +  gIMAPMailbox.specialUseFlag = "\\Inbox";
> +  gIMAPMailbox.subscribed = true;
> +  

whitespace

@@ +80,5 @@
> +  // need all mail folder to identify this as gmail server.
> +  gIMAPDaemon.createMailbox("[Gmail]", {flags : ["\\NoSelect"] });
> +  gIMAPDaemon.createMailbox("[Gmail]/All Mail", {subscribed : true,
> +                                                 specialUseFlag : "\\AllMail"});
> +  

whitespace

@@ +213,5 @@
> +
> +asyncUrlListener.callback = function(aUrl, aExitCode) {
> +  do_check_eq(aExitCode, 0);
> +};
> + 

whitespace

@@ +239,5 @@
>  
>      scriptStream.read(aCount);
>    }
>  };
> + 

whitespace

::: mailnews/imap/test/unit/test_listSubscribed.js
@@ +2,5 @@
> + * License, v. 2.0. If a copy of the MPL was not distributed with this
> + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
> +
> +// Test that imapd.js fakeserver correlecty implements LIST-EXTENDED imap
> +// extension (RFC 5258 - http://tools.ietf.org/html/rfc5258)

Update the comment for this test

@@ +55,5 @@
> +  let rootFolder = gIMAPIncomingServer.rootFolder;
> +  let folder1 = rootFolder.getChildNamed("folder1");
> +  do_check_true(folder1.getFlag(nsMsgFolderFlags.ImapNoselect));
> +  do_check_false(folder1.getFlag(nsMsgFolderFlags.ImapNoinferiors));
> +  

white space

::: mailnews/test/fakeserver/imapd.js
@@ +613,5 @@
>  
> +// used by RFC 5258 and GMail (labels)
> +function parseMailboxList(aList) {
> +
> +  // strip enclosing parenthises

parentheses

@@ +1121,5 @@
> +      }
> +    }
> +    if (!this[listFunctionName])
> +      return 'BAD unknown LIST request options';
> +      

whitespace
Comment 12 David Lechner (:dlech) 2012-10-30 19:20:36 PDT
Created attachment 676879 [details] [diff] [review]
related tests, etc. v3

Fixed nits. Thanks irving!

> Would it be worth the effort to extract the common setup and related code from
> all these tests? I'm not going to r- for it.
Yes, I think so. I plan on working on more related bugs, so I will take care of it on a later patch.
Comment 13 Ludovic Hirlimann [:Usul] 2012-10-31 03:16:02 PDT
What needs to be checked-in David ?
Comment 14 :Irving Reid (No longer working on Firefox) 2012-10-31 05:54:28 PDT
Can we hold off on checking this in for a bit? I may have just seen an issue with one of my IMAP accounts and I want to look into it.
Comment 15 :Irving Reid (No longer working on Firefox) 2012-10-31 06:04:55 PDT
When I start up TB on a profile that connects to Mozilla's Zimbra server I get a popup warning box containing:

The current operation on 'Inbox' did not succeed. The mail server for account ireid@mozilla.com responded: LIST failed: wildcards not permitted in username.

The relevant lines from an imap:5 log appear to be:

2012-10-31 12:56:23.172058 UTC - 596119552[11febe550]: 22ad4000:mail.mozilla.com:A:SendData: 9 list (subscribed) "" "/home/*"
2012-10-31 12:56:23.312700 UTC - 596119552[11febe550]: ReadNextLine [stream=22886a58 nb=55 needmore=0]
2012-10-31 12:56:23.312710 UTC - 596119552[11febe550]: 22ad4000:mail.mozilla.com:A:CreateNewLineFromSocket: 9 NO LIST failed: wildcards not permitted in username
Comment 16 Ben Bucksch (:BenB) 2012-10-31 07:24:36 PDT
(Outch. We really need a testsuite that contacts real-life servers, on a separate test machine with Internet access.)
Comment 17 David Lechner (:dlech) 2012-10-31 10:54:34 PDT
Created attachment 677063 [details] [diff] [review]
fix v2

Interesting. Looks like I have resurrected Bug 417285. Since we are using LIST command from Lsub function instead of List function. I applied the same fix to the Lsub function. Do we want to log to Activity Manager like mentioned in Bug 417285 Comment 11?
Comment 18 Ludovic Hirlimann [:Usul] 2012-11-01 02:01:11 PDT
Logging to activity manager should be for things that users understand. I'm not sure most user know what LIST and LSUB are. I'd say no. We could log it in NSPR though.
Comment 19 David Lechner (:dlech) 2012-11-01 08:33:17 PDT
(In reply to Ludovic Hirlimann [:Usul] from comment #18)
> Logging to activity manager should be for things that users understand. I'm
> not sure most user know what LIST and LSUB are. I'd say no. We could log it
> in NSPR though.

OK. Information is already in IMAP log for more ambitious users to find.
Comment 20 :Irving Reid (No longer working on Firefox) 2012-11-08 13:44:07 PST
(In reply to David Lechner (:dlech) from comment #17)
> Created attachment 677063 [details] [diff] [review]
> fix v2

Looks like you attached a copy of the tests patch, rather than the fix
Comment 21 David Lechner (:dlech) 2012-11-08 17:03:58 PST
Created attachment 679912 [details] [diff] [review]
fix v2

Oops. Here is the correct patch.
Comment 22 :Irving Reid (No longer working on Firefox) 2012-11-14 10:11:13 PST
Comment on attachment 679912 [details] [diff] [review]
fix v2

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

r+ after the whitespace nits are fixed. The test patch looks good too, but needs to be updated because xpcshell.ini has conflicting changes. I'd like to land both at the same time.

::: mailnews/imap/src/nsImapServerResponseParser.cpp
@@ +848,5 @@
> +        // RFC 5258 \NonExistent implies \Noselect
> +        boxSpec->mBoxFlags |= kNoselect;
> +      }
> +      else if (!PL_strncasecmp(fNextToken, "\\Subscribed", 10))
> +        boxSpec->mBoxFlags |= kSubscribed;  

Trailing white space

@@ +850,5 @@
> +      }
> +      else if (!PL_strncasecmp(fNextToken, "\\Subscribed", 10))
> +        boxSpec->mBoxFlags |= kSubscribed;  
> +      else if (!PL_strncasecmp(fNextToken, "\\Remote", 6))
> +        boxSpec->mBoxFlags |= kRemote;  

Trailing white space

@@ +852,5 @@
> +        boxSpec->mBoxFlags |= kSubscribed;  
> +      else if (!PL_strncasecmp(fNextToken, "\\Remote", 6))
> +        boxSpec->mBoxFlags |= kRemote;  
> +      else if (!PL_strncasecmp(fNextToken, "\\HasChildren", 11))
> +        boxSpec->mBoxFlags |= kHasChildren;  

Trailing white space
Comment 23 David Lechner (:dlech) 2012-11-14 10:29:43 PST
Created attachment 681556 [details] [diff] [review]
related tests, etc. v4
Comment 24 David Lechner (:dlech) 2012-11-14 10:30:27 PST
Created attachment 681558 [details] [diff] [review]
fix v3
Comment 25 David Lechner (:dlech) 2012-11-14 10:34:41 PST
Both patches need checked in. Tests v4 patch is r=bienvenu, Fix v3 patch is r=irving.
Comment 27 Mark Banner (:standard8, limited time in Dec) 2012-11-15 05:38:01 PST
Unfortunately, I've just had to back out the tests, because there appears to be permanent orange on Windows Opt builds, and frequent orange on Linux opt builds:

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

For example:

https://tbpl.mozilla.org/php/getParsedLog.php?id=17064593&tree=Thunderbird-Trunk

TEST-UNEXPECTED-FAIL | c:\talos-slave\test\build\xpcshell\tests\mailnews\base\test\unit\test_testsuite_fakeserver_imapd_gmail.js | test failed (with xpcshell return code: 0), see following log:
>>>>>>>
*******************************************
Generator explosion!
Unhappiness at: c:/talos-slave/test/build/xpcshell/tests/mailnews/base/test/unit/test_testsuite_fakeserver_imapd_gmail.js:46
Because: TypeError: gIMAPServer._readers[0] is undefined
Stack:
  setupMailboxes@c:/talos-slave/test/build/xpcshell/tests/mailnews/base/test/unit/test_testsuite_fakeserver_imapd_gmail.js:46
  _async_driver@../../../resources/asyncTestUtils.js:156
  .run@c:\talos-slave\test\build\xpcshell\head.js:418
  
**** Async Generator Stack source functions:
  setupMailboxes
  _async_test_runner
*********
TypeError: gIMAPServer._readers[0] is undefined
-- Exception object --
+ fileName (string) 'c:/talos-slave/test/build/xpcshell/tests/mailnews/base/test/unit/test_testsuite_fakeserver_imapd_gmail.js'
+ lineNumber (number) 46
+ stack (string) 231 chars
+ columnNumber (number) 2
*
-- Stack Trace --
setupMailboxes@c:/talos-slave/test/build/xpcshell/tests/mailnews/base/test/unit/test_testsuite_fakeserver_imapd_gmail.js:46
_async_driver@../../../resources/asyncTestUtils.js:156
.run@c:\talos-slave\test\build\xpcshell\head.js:418

TEST-UNEXPECTED-FAIL | ../../../resources/asyncTestUtils.js | Generator explosion. ex: [error TypeError] async stack [undefined undefined] - See following stack:
JS frame :: c:\talos-slave\test\build\xpcshell\head.js :: do_throw :: line 451
JS frame :: ../../../resources/logHelper.js :: mark_failure :: line 620
JS frame :: ../../../resources/asyncTestUtils.js :: _async_driver :: line 178
JS frame :: c:\talos-slave\test\build\xpcshell\head.js :: .run :: line 418
native frame :: <unknown filename> :: <TOP_LEVEL> :: line 0


TEST-UNEXPECTED-FAIL | c:\talos-slave\test\build\xpcshell\tests\mailnews\base\test\unit\test_testsuite_fakeserver_imapd_list-extended.js | test failed (with xpcshell return code: 0), see following log:
>>>>>>>
*******************************************
Generator explosion!
Unhappiness at: c:/talos-slave/test/build/xpcshell/tests/mailnews/base/test/unit/test_testsuite_fakeserver_imapd_list-extended.js:54
Because: TypeError: gIMAPServer._readers[0] is undefined
Stack:
  setupMailboxes@c:/talos-slave/test/build/xpcshell/tests/mailnews/base/test/unit/test_testsuite_fakeserver_imapd_list-extended.js:54
  _async_driver@../../../resources/asyncTestUtils.js:156
  .run@c:\talos-slave\test\build\xpcshell\head.js:418
  
**** Async Generator Stack source functions:
  setupMailboxes
  _async_test_runner
*********
TypeError: gIMAPServer._readers[0] is undefined
-- Exception object --
+ fileName (string) 'c:/talos-slave/test/build/xpcshell/tests/mailnews/base/test/unit/test_testsuite_fakeserver_imapd_list-extended.js'
+ lineNumber (number) 54
+ stack (string) 239 chars
+ columnNumber (number) 2
*
-- Stack Trace --
setupMailboxes@c:/talos-slave/test/build/xpcshell/tests/mailnews/base/test/unit/test_testsuite_fakeserver_imapd_list-extended.js:54
_async_driver@../../../resources/asyncTestUtils.js:156
.run@c:\talos-slave\test\build\xpcshell\head.js:418

TEST-UNEXPECTED-FAIL | ../../../resources/asyncTestUtils.js | Generator explosion. ex: [error TypeError] async stack [undefined undefined] - See following stack:
JS frame :: c:\talos-slave\test\build\xpcshell\head.js :: do_throw :: line 451
JS frame :: ../../../resources/logHelper.js :: mark_failure :: line 620
JS frame :: ../../../resources/asyncTestUtils.js :: _async_driver :: line 178
JS frame :: c:\talos-slave\test\build\xpcshell\head.js :: .run :: line 418
native frame :: <unknown filename> :: <TOP_LEVEL> :: line 0
Comment 28 David Lechner (:dlech) 2012-11-15 22:27:10 PST
Created attachment 682333 [details] [diff] [review]
attempt to fix try server fails

This will hopefully fix the fails on the try server. I left it as a separate patch for easy review. I will merge it into the other patch before checkin. I am also guessing that I should give this a go on the try server before requesting checkin again. Should I be asking for someone to do this for me or should I be requesting try server access so that I can do it myself (and if so, how do I go about doing this)?
Comment 29 Ludovic Hirlimann [:Usul] 2012-11-15 22:45:29 PST
You should be requesting try server access so you have them in the future.

It's documented at http://www.mozilla.org/hacking/committer/ - I don't know the level you'll need to access try servers.
Comment 30 Atul Jangra [:atuljangra] 2012-11-15 22:46:54 PST
David, you can get try server access easily. Read http://www.mozilla.org/hacking/committer/ for information.
Alternatively, I can do a try server run for you.
Comment 31 Atul Jangra [:atuljangra] 2012-11-15 22:47:48 PST
That would be L1 access, I guess. :)
Comment 32 Ludovic Hirlimann [:Usul] 2012-11-15 22:50:03 PST
(In reply to Atul Jangra [:atuljangra] from comment #30)

> Alternatively, I can do a try server run for you.

yes that would speed things up for this bug.
Comment 33 Atul Jangra [:atuljangra] 2012-11-15 22:51:22 PST
Cool so lets wait for irving's review, and then I'll do the try server run. :)
Comment 34 David Lechner (:dlech) 2012-11-15 22:53:26 PST
Atul, no need to wait for review. If you could run it now, then I can go to sleep and find out what happened in the morning :)

All 3 patches need to be pushed.
Comment 35 Atul Jangra [:atuljangra] 2012-11-16 04:07:28 PST
Pushed to try server: 
https://tbpl.mozilla.org/?tree=Thunderbird-Try&rev=960576197481

Hoping for good results :)
Comment 36 Atul Jangra [:atuljangra] 2012-11-16 06:09:11 PST
Results are out :-/
Build burning on Win64 opt and Linux Debug. And failure of two tests :(
https://tbpl.mozilla.org/?tree=Thunderbird-Try&rev=960576197481
Comment 37 Mark Banner (:standard8, limited time in Dec) 2012-11-16 06:20:28 PST
(In reply to Atul Jangra [:atuljangra] from comment #36)
> Results are out :-/
> Build burning on Win64 opt and Linux Debug. And failure of two tests :(
> https://tbpl.mozilla.org/?tree=Thunderbird-Try&rev=960576197481

Win64 is a known build issue, Linux debug is some sort of infra issue (upload failed, I've alerted buildduty), both of these have been starred & rebuilds requested. I think both the unit test failures are known issues, I've starred one that I know is, and I've requested a rebuild of the second to make sure.
Comment 38 Atul Jangra [:atuljangra] 2012-11-16 06:24:37 PST
Thanks :-) I hope everything goes well before dlech wakes up :)
Comment 39 Mozilla RelEng Bot 2012-11-16 07:45:38 PST
Try run for 960576197481 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=960576197481
Results (out of 22 total builds):
    success: 15
    warnings: 4
    failure: 3
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/atuljangra66@gmail.com-960576197481
Comment 40 Atul Jangra [:atuljangra] 2012-11-16 08:21:46 PST
Corrected links:
http://ftp.mozilla.org/pub/mozilla.org/thunderbird/try-builds/atuljangra66@gmail.com-960576197481/

and 

https://tbpl.mozilla.org/?tree=Thunderbird-Try&rev=960576197481


P.S.: Did we just found a bug about Mozilla RelEng Bot?
Comment 41 Mark Banner (:standard8, limited time in Dec) 2012-11-16 08:23:48 PST
(In reply to Atul Jangra [:atuljangra] from comment #40)
> P.S.: Did we just found a bug about Mozilla RelEng Bot?

Known issue with the bot, see bug 807778 (also linked to from https://wiki.mozilla.org/Thunderbird/Infrastructure/TryServer which is linked from the top of tbpl)
Comment 42 David Lechner (:dlech) 2012-11-16 09:12:40 PST
Atul, High five and thanks very much for the try server run!

Mark, Thanks for your input/assistance too!

If I am reading everything correctly, the Linux debug rebuild succeeded and the Win64 build can be ignored. So, I just need and r+ on the additional changes and then we can checkin again.
Comment 43 Atul Jangra [:atuljangra] 2012-11-16 09:16:43 PST
HI5 :-).
Ans yes, we are ready for a good checkin :)
Comment 44 David Lechner (:dlech) 2012-11-16 12:42:50 PST
For the record, the reason for the failures was that I was trying to cheat and grab an instance of an existing handler from the fakeserver imapd. I am guessing that in optimized builds that there was a timing issue and the handler had not been instantiated yet before I tried to reference it. 

To fix, I am now creating a new instance of a handler instead of trying to steal the existing one that may not exist yet. In order to do this, I had to move a few things from the .prototype section of the IMAP_RFC3501_handler to the constructor section in imapd.js. Object, etc. in .prototype are shared between all instances, so by having 2 handlers, extensions were getting mixed in twice and causing problems.
Comment 45 David Lechner (:dlech) 2012-11-19 14:09:21 PST
Created attachment 683304 [details] [diff] [review]
related tests, etc. v5

Merged 2 patches. 

This is the only patch that needs to be checked in now. The other patch was not backed out. r=irving
Comment 46 Ryan VanderMeulen [:RyanVM] 2012-11-19 17:54:37 PST
https://hg.mozilla.org/comm-central/rev/6996e05c74ee
Comment 47 Joe Sabash [:JoeS1] 2012-11-20 17:16:25 PST
Unless I missed something , the milestone should be tb20
Comment 48 David Lechner (:dlech) 2012-11-20 17:59:38 PST
Comment on attachment 683304 [details] [diff] [review]
related tests, etc. v5

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 683304
User impact if declined: 
Testing completed (on m-c, etc.): yes
Risk to taking this patch (and alternatives if risky): low
String or UUID changes made by this patch: none


As pointed out by Joe, this patch landed just after the latest aurora branch. However, the other patch for this bug was landed before the branch. So, that means there is a patch in aurora without any tests. So, it would be beneficial to have this patch merged into aurora as well.
Comment 49 David Lechner (:dlech) 2012-11-20 18:02:43 PST
Changing milestone back because the patch that actually fixes the bug was not backed out before the branch, only the tests.
Comment 50 :Irving Reid (No longer working on Firefox) 2012-11-24 10:56:41 PST
Comment on attachment 683304 [details] [diff] [review]
related tests, etc. v5

Test landed on Aurora:
https://hg.mozilla.org/releases/comm-aurora/rev/97e6f255cbc8
Comment 51 Dave Miller [:justdave] (justdave@bugzilla.org) 2012-11-24 19:50:09 PST
FWIW, I've been unable to connect to mozilla.com's Zimbra server since this patch landed.  I can confirm that older builds prior to 20121115 still work with it.  I think this is a bug in Zimbra's list (subscribed) support, and I've filed a ticket with Zimbra to investigate, but I thought I'd give you guys a heads-up.  The IMAP log from Thunderbird pretty clearly shows Zimbra hanging up after the list (subscribed) command is issued, and the logs on Zimbra's end show a crash and stack trace at that point, so I'm pretty positive this is Zimbra's fault and not Thunderbird's.  I'll follow up with a new bug if they try to pin it on Thunderbird. :)
Comment 52 Dave Miller [:justdave] (justdave@bugzilla.org) 2012-11-26 20:15:33 PST
It's been acknowledged as a bug in Zimbra, fix targeted for Zimbra 8.0.2 (they're not planning on backporting it to Zimbra 7.x at this point).

http://bugzilla.zimbra.com/show_bug.cgi?id=78794

Might want to keep an eye on this, and if they really don't backport it then do an exception for Zimbra 7.x to not use this? (the versions are clearly identified in the ID response)
Comment 53 Arnaud 2015-06-22 08:18:12 PDT
(In reply to Dave Miller [:justdave] (justdave@bugzilla.org) from comment #52)
> Might want to keep an eye on this, and if they really don't backport it then
> do an exception for Zimbra 7.x to not use this? (the versions are clearly
> identified in the ID response)

This just happened to one of our users, and we happen to be running Zimbra 7.2.7, which is the latest and last release of the 7.x branch. This won't be fixed on Zimbra, could this exception be implemented ?
Otherwise, is there a workaround for this problem ? Either server-side or client-side, but upgrading Zimbra is currently not an option.

Note You need to log in before you can comment on or make changes to this bug.