1466705 - Fix subscribe dialog for IMAP and news - continuation of bug 1425962

Status: RESOLVED FIXED

(MailNews Core :: General, enhancement)

Description

[Jorg K (CEST = GMT+2)]

+++ This bug was initially created as a clone of Bug #1425962 +++

In bug 1425962 we implemented a basic JS flat list subscribe. We need to fix this now and do a proper C++ tree again.

The approach is to use
https://hg.mozilla.org/users/Pidgeot18_gmail.com/patch-queues/file/tip/patches-derdf/subscribe-invert and https://hg.mozilla.org/users/Pidgeot18_gmail.com/patch-queues/file/tip/patches-derdf/subscribe-tree
as a starting point.

Comment 1

[Jorg K (CEST = GMT+2)]

Attachment: subscribe-invert.patch

Original from https://hg.mozilla.org/users/Pidgeot18_gmail.com/patch-queues/file/tip/patches-derdf/subscribe-invert.

Comment 2

[Jorg K (CEST = GMT+2)]

Attachment: subscribe-tree.patch

Original from https://hg.mozilla.org/users/Pidgeot18_gmail.com/patch-queues/file/tip/patches-derdf/subscribe-tree.

Comment 3

[Jorg K (CEST = GMT+2)]

Attachment: subscribe-tree.patch - JK v1: rebased and made to compile

Looks like this needs to be applied first.

No rocket science here, all this does is
- rename class nsSubscribableServer to SubscribableServer
  (which appears unnecessary)
- make SubscribableServer inherit from nsITreeView
  and provide the necessary methods, some of which are just dummies.

Comment 4

[Jorg K (CEST = GMT+2)]

Attachment: subscribe-tree.patch - JK v2: rebased and made to compile, reverted class rename

I renamed the class back to nsSubscribableServer. That cuts down the patch from 26KB to 14KB and removes needless noise.

Comment 5

[Jorg K (CEST = GMT+2)]

Attachment: subscribe-tree.patch - JK v2b: rebased and made to compile, reverted class rename

White-space tweaks.

Comment 6

[Jorg K (CEST = GMT+2)]

Attachment: subscribe-tree.patch - JK v2c: rebased and made to compile, reverted class rename

More white-space changes.

Comment 7

[Jorg K (CEST = GMT+2)]

Attachment: subscribe-invert2.patch - JK v1: rebased and made it compile

All hunks failed when applying the original patch. So I transferred all the hunks by hand. The code was really old, so I had to remove use of Atoms.

There are a few questions in the code left. So let's see what it does.

Comment 8

[Jorg K (CEST = GMT+2)]

Attachment: subscribe-invert2.patch - JK v2: rebased and made it compile and link

Let's see now ;-)

Comment 9

[Jorg K (CEST = GMT+2)]

OK, something is working, but I don't see a tree. I get a list if folders, but with the hierarchy separator, like:
INBOX/xyz/t. The folder names are not MUTF-7 decoded, so some look strange. And the check boxes can be clicked.

So somehow we need to split the paths at the separator, decode the names and assemble it all to a tree. The search doesn't work either.

Aceman do you want to have a go? You've go us to the point where the lists are displayed, so perhaps this all makes sense to you.

Comment 10

[Jorg K (CEST = GMT+2)]

Attachment: subscribe-invert2.patch - JK v2b: rebased and made it compile and link

Tweaked some comments after some experiments.

This actually shows a tree on news.mozilla.org :-)

Comment 11

[Jorg K (CEST = GMT+2)]

Attachment: subscribe-invert2.patch - JK v3: rebased and made it compile and link

This fixes the IMAP MUTF-7 issue. So at least folder names are looking OK now.

Still, the check boxes don't work.

Comment 12

[Jorg K (CEST = GMT+2)]

Attachment: subscribe-tree.patch

Merged both patches, added authors and commit message.

Aceman said via IRC that for the checkboxes to work we need (to revert?) some CSS changes.

Looking promising. With concentrated effort this could be done in a few days now.

Comment 13

[Jorg K (CEST = GMT+2)]

Attachment: subscribe-tree.patch - folded version v1b

Fixed messed-up comment that was accidentally hit by a global replace :-(

Comment 14

[:aceman]

Attachment: subscribe.patch - v1c

Fixes proper folder icons for news and tries to show full newsgroup names 9still a bit incorrectly). Cleans up some unneeded code.

Comment 15

[Jorg K (CEST = GMT+2)]

Attachment: subscribe.patch - v2

Removed some commented-out code, played a bit with pulling IMAP folder names apart. I'm still not sure what we need to do to create a tree.

Comment 16

[Jorg K (CEST = GMT+2)]

Current status:
- IMAP folders don't show up as tree.
- Checkboxes can't be checked.
- News server tree isn't shown properly when panel first loads, arrow indicates expanded, but it isn't.
  Quick fix: Always launch collapsed.
- Some dead JS code from bug 1425962, like createSubscribeTree().

Comment 17

[Jorg K (CEST = GMT+2)]

Attachment: subscribe.patch - v3

IMAP working now, just a delimiter problem. I'll post an interdiff link in the next comment so you can see it.

Comment 18

[Jorg K (CEST = GMT+2)]

https://bugzilla.mozilla.org/attachment.cgi?oldid=8984308&action=interdiff&newid=8984554&headers=1
Comparing v1c with v3, v2 was just plain wrong.

This is all that was needed for IMAP:
    char hierarchyDelimiter;
    GetDelimiter(&hierarchyDelimiter);
    nsCOMPtr<nsISubscribableServer> listener2 = do_QueryInterface(listener);
    if (listener2)
      listener2->SetDelimiter(hierarchyDelimiter);

Looks like there are two nsISubscribableServer objects being maintained. The second one didn't have the correct delimiter. So here its transferred from one object to the next.

Comment 19

[Jorg K (CEST = GMT+2)]

Attachment: subscribe.patch - v3b

This fixes the folder names for IMAP. So now IMAP and news have the same two common problems:
- Checkboxes can't be checked.
- Server tree isn't shown properly when panel first loads, arrow indicates expanded, but it isn't.
  Quick fix: Always launch collapsed.

Starting to look good. Over to Aceman now.

Comment 21

[:aceman]

Attachment: subscribe.patch - v4

This fixes the newsgroup names and also restores the italic font and no checkbox for unsubscribable rows (e.g. expandable nodes that do not represent a real newsgroup).

I also think there are still 2 trees being populated. You sync the delimiter and showFullName between the two. But all the other properties (like isSubscribable and isSubscribed) aren't synced. And that is why the checkboxes aren't ticked for subscribed newsgroups/folders. I have added addresses of the pointers to the node objects into the debugging printfs so that the duplicate objects can be seen.

I don't know why there need to be 2 trees, why there is the 'inner' member and also why the subscribeListener seems to implement the whole tree and why this code looks like a loop:
var inner = Cc["@mozilla.org/messenger/subscribableserver;1"]
              .createInstance(Ci.nsISubscribeListener)
              .QueryInterface(Ci.nsITreeView);
    gSubscribableServer.subscribeListener = inner;
    inner.QueryInterface(Ci.nsISubscribableServer)
         .subscribeListener = MySubscribeListener;
    inner.setIncomingServer(gSubscribableServer);

Also I do not see why listener->AddFolder() calls AddTo() again when that is done explicitly around the call to the AddFolder().

Comment 22

[Jorg K (CEST = GMT+2)]

Yes, agreed, this is a concern. When working on the sync for IMAP, I removed the listener->AddFolder() call and then the tree in the panel remained empty. I think it's a "display" tree and a "shadow" tree, but I don't know the reason for this distinction. Clearly, Joshua's patch adds the listener->AddFolder() calls and also
+    gSubscribableServer.subscribeListener = inner;
+    inner.QueryInterface(Ci.nsISubscribableServer)
+         .subscribeListener = MySubscribeListener;
+    inner.setIncomingServer(gSubscribableServer);
so it was his intent. So we might have to ask Joshua.

Comment 23

[Jorg K (CEST = GMT+2)]

Attachment: subscribe2.patch - v5 WIP -JK

Here's an attempt to remove Joshua's AddFolder() and tweak the AddTo() function so it always adds to the tree of the listener. Debug looks like:
=== AddTo(0000015522A91870): INBOX/INBOX
=== adding base
=== AddTo(0000015521F129D0): INBOX/INBOX
=== adding to listener (composed object)

So the server object is 0000015522A91870 and it's listener is 0000015521F129D0 and the AddTo() is forwarded to it.

Sadly nothing is displayed in the UI, so I must be missing something. There are also no calls to GetCell*() :-(

Comment 24

[Jorg K (CEST = GMT+2)]

Attachment: subscribe2.patch - v6 WIP -JK

Here's a version that shifts the action to the server tree. Maybe this is more promising. Sadly again, nothing on the screen :-(

Comment 25

[Jorg K (CEST = GMT+2)]

Comment on attachment 8985141 [details] [diff] [review]
subscribe2.patch - v6 WIP -JK

Oops, I missed
JavaScript error: chrome://messenger/content/subscribe.js, line 75: NS_NOINTERFACE: Component returned failure code: 0x80004002 (NS_NOINTERFACE) [nsISupports.QueryInterface]
on this line:
gSubscribeTree.view = gSubscribableServer.QueryInterface(Ci.nsITreeView);

So if we can't set the tree, then there won't be a display.

Comment 26

[:aceman]

Attachment: subscribe-servertree.patch v7 WIP

This is a continuation of v6, which seems to work.
It reduces the subscribelistener to the single function of OnDonePopulating and it does not implement the subscribableServer. The dialog tree UI is backed by the subscribableServer, that gets exposed from the nntp/imapincomingserver via the new folderView attribute.
In this patch this works now:
-display of the tree, also with accented folders in imap
-checkboxes for 'subscribed' work and you can correctly subscribe/unsubscribe from folders/newsgroups
-all the found folders show initially expanded (open) in the tree
-Refresh button works

This still has some rough edges:
1. console warnings
###!!! ASSERTION: row count did not change by the amount suggested, check caller: 'rowCount == mRowCount', file /var/NVME/TB-hg/mozilla/layout/xul/tree/nsTreeBodyFrame.cpp, line 1803

This is caused by improper use of mTree->RowCountChanged(). I have changed the order of some of these calls, but it does not seem enough. When this is messed up, it causes crashes of TB.

2. I remove some code in subscribe.js::SubscribeOnClick() that called  gSubscribableServer.startPopulatingWithUri(). I do not see we support any loading of parts of the tree. That is why I removed also the tree.getAttribute("ref") handling. Does anybody know what that was about?

Comment 27

[Jorg K (CEST = GMT+2)]

Comment on attachment 8985311 [details] [diff] [review]
subscribe-servertree.patch v7 WIP

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

::: mailnews/base/src/nsSubscribableServer.cpp
@@ +377,5 @@
>  nsSubscribableServer::StopPopulating(nsIMsgWindow *aMsgWindow)
>  {
> +  mStopped = true;
> +
> +printf("ACE: copy tree to listener, %p\n", (void*)mTree.get());

I hope not!!

Comment 28

[:aceman]

Yeah, the text is now outdated. It just builds the mRowMap still inside the same nsSubscribableServer object.

Comment 29

[Jorg K (CEST = GMT+2)]

This looks very nice :-) - Some comments:
- I'd like to run this without the console debug prints to get a feel for the speed
  on news servers.
- I subscribed to two folders which were unsubscribed and crashed when hitting the "Subscribe" button.
- I'll add some folders with CJK names to my test server.

Comment 30

[Jorg K (CEST = GMT+2)]

(In reply to Jorg K (GMT+2) from comment #29)
> - I subscribed to two folders which were unsubscribed and crashed when
> hitting the "Subscribe" button.
Now it crashed on "OK". Otherwise, very nice-ö ;-)

Comment 31

[:aceman]

Attachment: subscribe-servertree.patch v7.1 WIP

This shouldn't crash now.

I see a small glitch that the folders are shown in a different order in the folder pane than in the subscribe dialog. In the folder pane we use locale aware ordering, which isn't available in the c++ backend.

Comment 32

[:aceman]

(In reply to Jorg K (GMT+2) from comment #29)
> This looks very nice :-) - Some comments:
> - I'd like to run this without the console debug prints to get a feel for
> the speed on news servers.

I'll remove them after jcranmer likes the solution :)
You can of course disable them locally.

> - I subscribed to two folders which were unsubscribed and crashed when
> hitting the "Subscribe" button.

Should be fixed in the v7.1.

> - I'll add some folders with CJK names to my test server.

Thanks, they display Chinese/Japanese characters so it seems to work.

Comment 33

[Jorg K (CEST = GMT+2)]

(In reply to :aceman from comment #32)
> I'll remove them after jcranmer likes the solution :)
You might be disappointed, but I won't wait until likes it. I'll land this pretty much now.

Comment 34

[Jorg K (CEST = GMT+2)]

Attachment: subscribe-servertree.patch v7.1 but without the dump/printf statements

Comment 35

[Jorg K (CEST = GMT+2)]

Attachment: subscribe-servertree.patch v7.2 but without the dump/printf statements

Hmm, how did NS_ASSERT() compile for you, since it's NS_ASSERTION() ??

Comment 36

[Jorg K (CEST = GMT+2)]

Attachment: subscribe-servertree.patch v7.3

Fixed indentation in one spot.

Comment 37

[Jorg K (CEST = GMT+2)]

Attachment: 1466705-reenable-test.patch

Test dictated to me via IRC by Aceman. Works too now ;-)

Comment 38

[Jorg K (CEST = GMT+2)]

Comment on attachment 8985479 [details] [diff] [review]
subscribe-servertree.patch v7.3

Let's go with this, thanks for the collaboration :-)

Comment 39

[Jorg K (CEST = GMT+2)]

I did a try run:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=93028c153e4b19c5292535dd184bffd466acefc6

Sadly, one test failure:
TEST-UNEXPECTED-FAIL | comm/mailnews/news/test/unit/test_internalUris.js
The log says that it timed out, which is also what I see locally.
Most likely due to the test doing it's own listener:
function* test_grouplist() {
  // This tests nsNntpService::GetListOfGroupsOnServer
  let subserver = localserver.QueryInterface(Ci.nsISubscribableServer);
  let subscribeListener = {
    OnDonePopulating: function () { async_driver(); }
  };
  subserver.subscribeListener = subscribeListener;
Maybe that doesn't get called.

There's also a crash in Mozmill. Look for "Application unexpectedly closed" in
https://taskcluster-artifacts.net/dKc07SbWT4ueytRg6LZ0WQ/0/public/logs/live_backing.log
I've retriggered the job to see whether that's a random failure.

Comment 40

[Jorg K (CEST = GMT+2)]

Comment on attachment 8985479 [details] [diff] [review]
subscribe-servertree.patch v7.3

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

::: mailnews/imap/src/nsImapIncomingServer.cpp
@@ -2505,5 @@
> -  nsCOMPtr<nsISubscribeListener> listener;
> -  (void) GetSubscribeListener(getter_AddRefs(listener));
> -
> -  if (listener)
> -    listener->OnDonePopulating();

Why did you remove this?

::: mailnews/news/src/nsNntpIncomingServer.cpp
@@ -1123,5 @@
> -  if (!listener)
> -    return NS_ERROR_FAILURE;
> -
> -  rv = listener->OnDonePopulating();
> -  NS_ENSURE_SUCCESS(rv,rv);

And this?

Comment 41

[Jorg K (CEST = GMT+2)]

(In reply to Jorg K (GMT+2) from comment #39)
> There's also a crash in Mozmill. [snip]
> I've retriggered the job to see whether that's a random failure.
That didn't repeat, but the Xpcshell test failure is real :-(

I tried moving mSubscribeListener->OnDonePopulating(); to before the !mTreeRoot check in nsSubscribableServer::StopPopulating(), but that didn't help. I'm not sure why you moved the OnDonePopulating() calls from the IMAP/NNTP code to nsSubscribableServer.cpp.

Comment 42

[:aceman]

Yes, I moved those calls OnDonePopulating into nsSubscribableServer.cpp::StopPopulating as a way to merge common code and call it from the object that manages the listener (nothing else in imap/nntpincomingserver touches the listener now). Does it change anything to put them back?

Comment 43

[Jorg K (CEST = GMT+2)]

Further code nits:
I changed the assert to:
NS_ASSERTION(mRowMap.Length() == 0, "mRowMap should be empty");
Not "should have been".

Also
-  rv = mInner->AddTo(aName, addAsSubscribed, aSubscribable, changeIfExists);
-  NS_ENSURE_SUCCESS(rv,rv);
-
-  return rv;
+  return mInner->AddTo(aName, addAsSubscribed, aSubscribable, changeIfExists);
is more elegant, but you won't get the benefit of printing an error if AddTo() fails.

Comment 44

[Jorg K (CEST = GMT+2)]

Attachment: subscribe-servertree.patch v7.4

Shifted the OnDonePopulating() calls back to NNTP and IMAP.

Instead of timing out, the test now fails on
test_internalUris.js:159

  let groups = enumGroups("");
  for (let group in daemon._groups)
159 Assert.ok(groups.indexOf(group) >= 0);

I guess it's the same problem that we needed to address in the Mozmill test, that is, that things may not be ready yet.

Comment 45

[:aceman]

I think it's because the test is still broken as it uses the RDF items from the server, which are now cut off (the whole RDF infra in nsSubscribeServer is now ignored, whether it Works or not).
I have this fixed in https://bug457333.bmoattachments.org/attachment.cgi?id=8944167 but as we also removed OnItemDiscovered() I need to find another way how to access the newsgroups found.

> Shifted the OnDonePopulating() calls back to NNTP and IMAP.

As this calls the listener first, it just hides the fact mInner->StopPopulating() is not running correctly. I would be against this change untill we understand what is going on. As I think calling OnDonePopulating from inside StopPopulating should be the same. Even more correct, as we announce being done only after all the work is really done. With your change (and the old code), the listener was run before the nsSubscribable server had all the mRowMap populated. Which may have been fine in the past (as  nsSubscribable was NOT the backend for the tree display), but now it is.

Comment 46

[Jorg K (CEST = GMT+2)]

(In reply to :aceman from comment #45)
> > Shifted the OnDonePopulating() calls back to NNTP and IMAP.
> As this calls the listener first, it just hides the fact
> mInner->StopPopulating() is not running correctly. I would be against this
> change until we understand what is going on.
Well, I've just restored the previous state which BTW changes the behaviour. Now the test fails elsewhere and doesn't hang/timeout. But yes, understanding what's going on it best :-)

Comment 47

[:aceman]

But reverting only that part may seem to fix this particular call site, but may be logically incorrect with respect to the other changes done to the subscribableServer backend. And will blow up elsewhere.
So please do not land it until I check it out in the evening.

Comment 48

[Jorg K (CEST = GMT+2)]

Not landing anything ;-)

Comment 49

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/ec2cc16327fe
Restore IMAP and NNTP subscribe to use a C++ nsITreeView. r=jorgk,aceman

Comment 50

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/f41fe8a63f2b
Backed out changeset ec2cc16327fe which was pushed accidentally. a=backout DONTBUILD

Comment 51

[Jorg K (CEST = GMT+2)]

Attachment: subscribe-servertree.patch v7.3b

I had a log discussion with Aceman on IRC about the test failure. The test calls GetListOfGroupsOnServer() which runs the URL and finally ends up in nsNntpIncomingServer::OnStopRunningUrl() which calls nsNntpIncomingServer::StopPopulating() and thus nsSubscribableServer::StopPopulating(). The latter returns straight away, since nsSubscribableServer::StartPopulating() was never called.

v7.3b fixes that by adding a call of nsSubscribableServer::StartPopulating() to nsNntpIncomingServer::OnStartRunningUrl().

Like this, the test passes.

v7.3b also fixes the nits from comment #43, so do an interdiff of v7.3b with v7.3. Forget v7.4 ;-)

Comment 52

[Jorg K (CEST = GMT+2)]

Attachment: subscribe-servertree.patch v7.3c

Oops, Aceman had some white-space correction in his v7.5 from
https://hg.mozilla.org/try-comm-central/rev/f86b331d677d88ec13ad3c9ae22d3602e88a6a95
which I've incorporated here.

Aceman, please check that this added
  nsresult rv = mInner->StartPopulating(nullptr, /* ignored */ false, /* ignored */ false);
doesn't do any damage ;-)

Comment 53

[Jorg K (CEST = GMT+2)]

I noticed another problem. If I click "Subscribe" or "Unsubscribe", the check mark isn't set/removed immediately. I need to hover the cell to see it update.

Comment 54

[Jorg K (CEST = GMT+2)]

Attachment: subscribe-servertree-7.5.patch v7.5

Attaching Aceman's
https://hg.mozilla.org/try-comm-central/rev/f86b331d677d88ec13ad3c9ae22d3602e88a6a95
for easy comparison.

Comment 55

[Jorg K (CEST = GMT+2)]

So Aceman, we have two working versions with little difference
https://bugzilla.mozilla.org/attachment.cgi?oldid=8985736&action=interdiff&newid=8985754&headers=1
Which one do we choose?

Comment 56

[:aceman]

(In reply to Jorg K (GMT+2) from comment #52)
> Created attachment 8985736 [details] [diff] [review]
> subscribe-servertree.patch v7.3c
> 
> Oops, Aceman had some white-space correction in his v7.5 from
> https://hg.mozilla.org/try-comm-central/rev/
> f86b331d677d88ec13ad3c9ae22d3602e88a6a95
> which I've incorporated here.
> 
> Aceman, please check that this added
>   nsresult rv = mInner->StartPopulating(nullptr, /* ignored */ false, /*
> ignored */ false);
> doesn't do any damage ;-)

Sorry, I can't decide this and we will be drifting far from the intent of this bug.
If you want to change the behaviour of GetListOfGroupsOnServer() or even remove it from public API, I'm fine with that, but we can't do it this quickly as we do not fully understand it. We should do it in a separate bug.

Comment 57

[Jorg K (CEST = GMT+2)]

Comment on attachment 8985736 [details] [diff] [review]
subscribe-servertree.patch v7.3c

OK, let's go with 7.5, but please fix comment #53.

Comment 58

[Jorg K (CEST = GMT+2)]

Attachment: subscribe-servertree.patch v7.6

Aceman's v7.5 plus fixing the refresh issue from comment #53 as dictated by Aceman via IRC.

Comment 59

[Jorg K (CEST = GMT+2)]

Attachment: subscribe-servertree.patch v7.6b

Oops, don't call invalidate on negative index.

Comment 60

[Jorg K (CEST = GMT+2)]

Attachment: subscribe-servertree.patch v7.7

Aceman's latest version fresh from the try server with some tweaks:
Changed strings a little, Cu.reportError() doesn't need a trailing \n.

Sadly I don't see neither the Cu.reportError() nor the status message :-( - I see "Please wait...".

Comment 61

[Jorg K (CEST = GMT+2)]

(In reply to Jorg K (GMT+2) from comment #60)
> Sadly I don't see neither the Cu.reportError() nor the status message :-( -
> I see "Please wait...".
Actually, when I go offline and do subscribe, I see that message.

So should we go with this now or do we want another iteration?

How do I port the new strings to TB 60:
+offlineState=You are offline. Items could not be retrieved from the server.
+errorPopulating=Error retrieving items from the server.

Aceman suggested to hack the former using:
https://searchfox.org/comm-central/rev/0d312c74b85ff3292247dddb2217f50ab0df45f3/mail/locales/en-US/chrome/messenger/offline.properties#28
and drop the second one which shouldn't show up?
So I'd have to add offline.properties to subscribe.xul to get to the string bundle.

Comment 62

[Jorg K (CEST = GMT+2)]

Looking for subscribe bugs currently yields 83 of them: https://mzl.la/2JIKpop

The blank panel when no password is available since no folder was accessed yet is bug 534994 :-(

Comment 63

[Jorg K (CEST = GMT+2)]

Attachment: subscribe-servertree.patch v8.0

Final patch \o/:

Removed class nsSubscribeListener since it wasn't used any more and moved Cu.reportError() to the else branch.

Ready to go, but we don't fix all the other 83 bugs here :-(

Comment 64

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/194e28dafcb5
Restore IMAP and NNTP subscribe to use a C++ nsITreeView. r=jorgk,aceman
https://hg.mozilla.org/comm-central/rev/ce10f3ede4cc
Re-enable test-subscribe-news-filter.js. r=jorgk

Comment 65

[Jorg K (CEST = GMT+2)]

(In reply to Jorg K (GMT+2) from comment #61)
> So should we go with this now or do we want another iteration?
I'd land this, but without the Cu.reportError(). What do you think?

Sadly there's another issue:

Comment 66

[Jorg K (CEST = GMT+2)]

Grrrr, Form History Control posted the previous comment :-(

Comment 67

[Jorg K (CEST = GMT+2)]

Attachment: beta.patch

For beta we need to
- s/nsTreeColumn/nsITreeColumn/
- s/mozilla::dom::DataTransfer/nsIDOMDataTransfer/
- hack the offline string.

Comment 68

[Jorg K (CEST = GMT+2)]

Beta try:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=3d907260e7db72bcf3b9131139011611d9b98f0f

Comment 69

[Jorg K (CEST = GMT+2)]

TB 60 beta 8 (BETA_60_CONTINUATION branch):
https://hg.mozilla.org/releases/comm-beta/rev/f87c98a8bb92efc0eb490d9721983315e920a396
https://hg.mozilla.org/releases/comm-beta/rev/df5d0c0ef77f45e9f8a0af7b6cc960f02ac36ba9

Accidentally pushed with new strings, to removing them again here:
https://hg.mozilla.org/releases/comm-beta/rev/444d81aa50552021164a8dcc962c566676ddedc6

Comment 70

[Landry Breuil (:gaston)]

IMAP subscribe tree works fine in 60.0b7 + rev f87c98a8bb92efc0eb490d9721983315e920a396 backported on top, on OpenBSD/amd64. Thanks!

Comment 71

[Jorg K (CEST = GMT+2)]

Glad you like it. As per comment #62, subscribe has some issues :-(

Comment 72

[Jorg K (CEST = GMT+2)]

https://hg.mozilla.org/releases/comm-esr60/rev/6953e664c375
https://hg.mozilla.org/releases/comm-esr60/rev/5950d54bde78
https://hg.mozilla.org/releases/comm-esr60/rev/ce00a781468c