After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 519292 - Threading is always collapsed, sort by date does not work anymore, "expand all" fails
Threading is always collapsed, sort by date does not work anymore, "expand al...
Status: RESOLVED FIXED
Product: evolution
Classification: Applications
Component: Mailer
2.22.x (obsolete)
Other All
: Normal normal
: ---
Assigned To: Milan Crha
Evolution QA team
muelli[thread]
: 484987 515192 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2008-02-28 14:41 UTC by Michael Kopp
Modified: 2008-08-12 12:37 UTC
See Also:
GNOME target: ---
GNOME version: 2.21/2.22


Attachments
proposed evo patch (3.31 KB, patch)
2008-03-06 20:03 UTC, Milan Crha
committed Details | Review
proposed evo patch (8.16 KB, patch)
2008-08-08 12:28 UTC, Milan Crha
committed Details | Review
proposed evo patch (1.90 KB, patch)
2008-08-12 08:16 UTC, Milan Crha
committed Details | Review

Description Michael Kopp 2008-02-28 14:41:50 UTC
Please describe the problem:
I used the 'Group by Threads' feature.

Up until 2.21.91 Threads were always expanded at startup. A new mail coming to an old thread would push that thread to the top of my Mail View. I sort by date descending so this is exactly what I want. It takes the newest date in the thread to sort by.

In 2.21.92 this changed:

1. On startup all threads are always collapsed. even if I do expand all, the next start will collapse all threads again
2. Order by date does not work anymore. It always takes the first email in the thread to order. that makes threading much less useful! new emails for older threads are not on top which makes them easy to miss.

Steps to reproduce:
1. 
2. 
3. 


Actual results:


Expected results:
Either switch this back the old behavior or make it configurable somehow please.

Does this happen every time?


Other information:
Comment 1 Daniel Gryniewicz 2008-03-05 16:17:02 UTC
This happens to me, too.  It means you cannot read new mail without manually expanding threads, as expand-all-threads does not actually expand all threads.
Comment 2 Milan Crha 2008-03-06 19:29:36 UTC
Hi, 
a) we've there a "secret" option (it only doesn't have its UI widget) to fall back to old sorting (which you name as "new"), means do not use most recent mail in the thread to sort them. It seems that the scheme didn't install for you or something similar. Add/change boolean value in /apps/evolution/mail/display/thread_latest and set it to true, which is the default by scheme (in case the scheme will not be installed properly, it uses "false", which explains your behavior.

b) Please check what is your value for /apps/evolution/mail/display/thread_expand
It is used as default expanded state for new messages. It doesn't have its widget too, hmm. I'm not sure whether leaf mail will be "expanded" properly on Expand All Threads, so any new sub message will be shown, I've such a feeling it will work weird, but only a feeling.

c) Confirming, View->Collapse All Threads doesn't work properly, but Expand All Threads does work well for me, probably related to those two options above. Can you check that, please?
Comment 3 Milan Crha 2008-03-06 20:03:29 UTC
Created attachment 106712 [details] [review]
proposed evo patch

for evolution;

This is for c) above, it should fix it. One note: the ./mail/ part is just a clean up, it doesn't have much to do with this bug, somehow.

I strongly believe we have this bug already filled. If you can try this patch and probably confirm it works and then find that bug and mark as a duplicate of this, then it will be great. Thanks.
Comment 4 Michael Kopp 2008-03-07 10:51:42 UTC
I checked the two secret settings, they were indeed missing in my config. after setting them i'm back at the 'old' behaviour. We should really have widgets for those.
Comment 5 Daniel Gryniewicz 2008-03-07 16:57:57 UTC
Setting the two secret settings worked for me, too, but only after dumping evolution to backup and loading it into a new account.  Not sure what made the difference here, but I'm working again.
Comment 6 Tobias Mueller 2008-03-10 14:21:44 UTC
*** Bug 515192 has been marked as a duplicate of this bug. ***
Comment 7 Matthew Barnes 2008-03-11 00:36:40 UTC
Bumping version to a stable release.
Comment 8 Srinivasa Ragavan 2008-03-27 08:56:05 UTC
Milan, message-list.h changes are missing in the patch. Just add that and commit to stable/trunk.
Comment 9 Milan Crha 2008-03-27 11:46:25 UTC
Committed to trunk. Committed revision 35271.
Committed to gnome-2-22. Committed revision 35272.

Hmm, I see there that change, it's the first chunk in the patch.
Comment 10 Patrick Ohly 2008-08-01 13:44:33 UTC
Asking users to set the hidden gconf values in order to get the previous behavior (threads expanded by default) back is not a suitable solution for the problem. Changing the default behavior might make sense when the new default is more useful, but in this case this change wasn't intentional: the default in evolution/mail/evolution-mail.schemas.in has "true" as default for thread_expand while the code in evolution/mail/message-list.c seems to get a "false" if the key has not be set yet.

Upgrading Evolution (regardless whether compiled from source or via a distribution package manager) does not update user gconf databases. Therefore all users upgrading from Evolution < 2.12 to >= 2.12 will be affected.

Besides changing the user behavior, not having thread_expand set to true also seems to cause problems with the "expand all threads" operation, as Milan Crha confirmed in an email discussion that I had with him.

I'm therefore reopening this bug.
Comment 11 Patrick Ohly 2008-08-01 13:45:22 UTC
*** Bug 484987 has been marked as a duplicate of this bug. ***
Comment 12 Patrick Ohly 2008-08-01 13:50:34 UTC
This was my description from bug #484987:
The menu item "view/expand all threads" does not expand all threads: some
threads are completely expanded, in others only the top-level is expanded.

Milan wrote in an email:
Yes, I have that [= thread_expand] set to true and it works just fine.

I tried set it to false and then the things became to happen. I can see
some strangeness, also "expand all" not working properly for some mails;
changing filter in the folder forth and back after expand all will
forgot the state and all of them are collapsed again. And maybe some
other I just didn't notice.
Comment 13 Milan Crha 2008-08-08 08:46:13 UTC
There is a bug #352695 about forgetting collapse state, so keep this for expand/collapse all only.
Comment 14 Milan Crha 2008-08-08 12:28:41 UTC
Created attachment 116135 [details] [review]
proposed evo patch

for evolution;

I dropped part of the previous patch, as well as the internal API of the ETree to collapse/expand all nodes. The problem was that the command has been called on a hash table of nodes shown in the tree, and each expand added new nodes to the hash table, same as collapse removed nodes from it. Other thing was that we firstly created the tree and then tried to change expanded state from the default value as requested. I changed this, the desired state is forced to overwrite the default value from the model and thus we traverse the list only once, not twice.
Comment 15 Srinivasa Ragavan 2008-08-11 05:00:29 UTC
Commit to trunk. Looks fine.
Comment 16 Milan Crha 2008-08-11 10:08:39 UTC
Committed to trunk. Committed revision 35954.
Comment 17 Patrick Ohly 2008-08-11 11:22:09 UTC
Perhaps I am missing something, but I don't find anything in the committed patch which addresses the problem mentioned in comment #10 ("upgrading Evolution changes the default behavior because gconfd variables are not set"). Reopening, please close if I'm wrong about that.

Comment 18 Milan Crha 2008-08-11 11:39:16 UTC
Hmm, you are not wrong about that, actually. Would you mind to create such patch? I guess it's only about catching the unset case and set values to TRUE for thread_expand and thread_latest.
Comment 19 Patrick Ohly 2008-08-11 20:31:19 UTC
[wrong defaults after upgrade]
> Hmm, you are not wrong about that, actually. Would you mind to create such
> patch?

Sorry, I don't have the time.

> I guess it's only about catching the unset case and set values to TRUE
> for thread_expand and thread_latest.

I'm not sure whether the gconf API for booleans let's the caller detect whether the value was set or not. If that's the case then a) these config options might have to be removed again in favor of hard-coded behavior (as you indicated earlier) and b) all new booleans should be defined so that "false" is the default (because that's what the gconf call seems to return).
Comment 20 Milan Crha 2008-08-12 08:06:47 UTC
(In reply to comment #19)
> [wrong defaults after upgrade]
> > Hmm, you are not wrong about that, actually. Would you mind to create such
> > patch?
> 
> Sorry, I don't have the time.

No problem at all, I'll do that.

> > I guess it's only about catching the unset case and set values to TRUE
> > for thread_expand and thread_latest.
> 
> I'm not sure whether the gconf API for booleans let's the caller detect whether
> the value was set or not. If that's the case then a) these config options might
> have to be removed again in favor of hard-coded behavior (as you indicated
> earlier) and b) all new booleans should be defined so that "false" is the
> default (because that's what the gconf call seems to return).
 
You've right, we were discussing removing the option, but I changed my mind, I would like to keep them, in case someone is using them (even I doubt a bit). The thing is this affects only users which compiles their own version and installation of the scheme file fails. Ordinary users should not be affected. With respect to b), yeah, that's a good idea.
Comment 21 Milan Crha 2008-08-12 08:16:38 UTC
Created attachment 116402 [details] [review]
proposed evo patch

for evolution;

ensures TRUE for thread_expand and thread_latest in case keys doesn't exits.
Feel free to reject and close if you think the self-compiling users should take care themself. :)
Comment 22 Patrick Ohly 2008-08-12 09:12:14 UTC
Patch looks good.

Regarding whether this affects all users who update or only self-compiling users: we've been through this before in a private email exchange. I still very much doubt that an update via a distribution will magically fix all users' gconf database by installing the new schema there. It will install the schema in the place where Evolution can find it for new setups, but it won't touch files in a user's /home. It might not even be *able* to, for example when the user has an encrypted or remotely mounted home and someone else is doing the package update. The gconf database is not accessible in that case.

Therefore I believe that this will affect *all* users who upgrade from a version which didn't have these config options.
Comment 23 Srinivasa Ragavan 2008-08-12 10:18:41 UTC
Commit to trunk
Comment 24 Milan Crha 2008-08-12 12:37:39 UTC
Committed to trunk. Committed revision 35968.