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 708004 - Deleting in threaded mode moves cursor to incorrect message
Deleting in threaded mode moves cursor to incorrect message
Status: RESOLVED FIXED
Product: evolution
Classification: Applications
Component: Mailer
3.10.x (obsolete)
Other Linux
: Normal normal
: ---
Assigned To: evolution-mail-maintainers
Evolution QA team
: 702007 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2013-09-13 05:29 UTC by David Dillow
Modified: 2014-11-19 13:06 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Select proper message on delete for 3.8.5 (592 bytes, patch)
2013-09-13 05:29 UTC, David Dillow
none Details | Review
Select proper message on delete for master (663 bytes, patch)
2013-09-13 05:30 UTC, David Dillow
committed Details | Review

Description David Dillow 2013-09-13 05:29:21 UTC
Created attachment 254829 [details] [review]
Select proper message on delete for 3.8.5

When deleting multiple messages from the message list in threaded mode, the message selected after the deletion is incorrect. The user expects that the immediately following message will be selected (barring thread reflowing) but evolution will select the Nth message after that, with N being the number of messages deleted.

ie, with the following in the message tree, with threading turned on:

Msg 1
 + Msg 2
 + Msg 3
Msg 4
Msg 5
Msg 6

Selecting Msg 1 to Msg 3 (using CTRL, or SHIFT, doesn't matter) and then deleting will cause Msg 6 to selected after the operation, when Msg 4 should be.

This issue does not occur if the message tree is not threaded, or if deleted messages are displayed.

This worked properly in 3.4.4, was broken in 3.6.4 and 3.8.5, and I suspect it is still broken in master based on code inspection.

In mail/message-list.c:build_tree() gets the proper next message from find_next_selectable() -- Msg 6 -- but its call to e_tree_set_cursor() is overwritten before before returning to message_list_regen_done_cb(), so that function attempts to set the cursor to the last row viewed (message_list->cursor_uid is NULL).

The reason that e_tree_set_cursor() is ineffective is that the call to message_list_tree_model_freeze() saves the current tree cursor (NULL) via e-util/e-tree-selection-model.c:etsm_pre_change(), and restores it when we call message_list_tree_model_thaw() via etsm_node_changed() and restore_cursor().

I've attached patches for 3.8.5 and master that moves the e_tree_set_cursor() outside of the freeze/thaw pairing. While it seems to work for me, I'm not 100% sure it is the proper fix.
Comment 1 David Dillow 2013-09-13 05:30:12 UTC
Created attachment 254830 [details] [review]
Select proper message on delete for master
Comment 2 Matthew Barnes 2013-09-14 17:19:55 UTC
*** Bug 702007 has been marked as a duplicate of this bug. ***
Comment 3 Matthew Barnes 2013-09-14 17:51:25 UTC
Excellent investigation, I can confirm the patch works.

Thanks a ton for figuring that out!  I actually tried to investigate it a few months ago but got lost in the signal handlers and missed the obvious issue.

Patch committed for Evolution 3.9.92:
https://git.gnome.org/browse/evolution/commit/?id=3d0ffdb864e7f1692b01e7e9a1adec37a735e704
Comment 4 Emmanuel Pacaud 2014-04-05 17:23:51 UTC
It looks this bug reappeared in evolution 3.10.4.
Comment 5 Milan Crha 2014-11-19 13:06:58 UTC
(In reply to comment #4)
> It looks this bug reappeared in evolution 3.10.4.

You are right, there had been couple reincarnations of this bug after the commit from comment #3. One of the latest I know of is bug #731278, which was fixed for evolution 3.12.4+. There can be even more (even indirectly) related bug fixes, but I think that the current 3.12.8 stable release handles this properly. Please reopen if I'm wrong.