Bug 11839

Summary: Show the previous e-mail after deleting an e-mail in VIEW mode
Product: [Extras] Maemo 5 Community SSU Reporter: Aniello Del Sorbo <anidel>
Component: modestAssignee: unassigned <nobody>
Status: VERIFIED FIXED QA Contact: general
Severity: enhancement    
Priority: Low CC: andrew, mohammad7410, naikel
Version: unspecifiedKeywords: patch
Target Milestone: ---   
Hardware: N900   
OS: Maemo   
Attachments: Patch to allow changing which message to show after deleting/moving a message from the VIEW window
Full patch to allow changing which message to show after deleting/moving a message from the VIEW window. Includes UI to change setting in Settings.

Description Aniello Del Sorbo (reporter) 2011-01-31 18:47:32 UTC
SOFTWARE VERSION: 
PR1.3

EXACT STEPS LEADING TO PROBLEM: 
(Explain in detail what you do (e.g. tap on OK) and what you see (e.g. message
Connection Failed appears))
1. Receive new e-mails
2. Show first received e-mails (meaning the oldest new received)
3. Delete e-mail

EXPECTED OUTCOME: 
Actually: PREFERRED OUTCOME -> "next" unread e-mail shows up for review

ACTUAL OUTCOME: 
"previous" read e-mail shows up, forcing a press to "<"

REPRODUCIBILITY: 
always

EXTRA SOFTWARE INSTALLED: 
Doesn't matter

OTHER COMMENTS: 
I would like to create a patch for this.
The code to be changed is in src/modest-ui-actions.c:

diff --git a/src/modest-ui-actions.c b/src/modest-ui-actions.c
index 6009122..2de4eeb 100644
--- a/src/modest-ui-actions.c
+++ b/src/modest-ui-actions.c
@@ -368,8 +368,8 @@ void
 modest_ui_actions_refresh_message_window_after_delete (ModestMsgViewWindow*
win
 {
        /* Close msg view window or select next */
-       if (!modest_msg_view_window_select_next_message (win) &&
-           !modest_msg_view_window_select_previous_message (win)) {
+       if (!modest_msg_view_window_select_previous_message (win) &&
+           !modest_msg_view_window_select_next_message (win)) {
                gboolean ret_value;
                g_signal_emit_by_name (G_OBJECT (win), "delete-event", NULL,
&re
        }

But I'd like to make this patch gconf-igurable.
Comment 1 Aniello Del Sorbo (reporter) 2011-01-31 18:50:41 UTC
Thanks to Sc0rpius for noticing.
Patched the comment as well:

diff --git a/src/modest-ui-actions.c b/src/modest-ui-actions.c
index 6009122..4fd347e 100644
--- a/src/modest-ui-actions.c
+++ b/src/modest-ui-actions.c
@@ -362,14 +362,14 @@ headers_action_mark_as_unread (TnyHeader *header,
 }

 /** After deleing a message that is currently visible in a window,
- * show the next message from the list, or close the window if there are no
more messages.
+ * show the previous message from the list, or close the window if there are
no more messages.
  **/
 void
 modest_ui_actions_refresh_message_window_after_delete (ModestMsgViewWindow*
win)
 {
        /* Close msg view window or select next */
-       if (!modest_msg_view_window_select_next_message (win) &&
-           !modest_msg_view_window_select_previous_message (win)) {
+       if (!modest_msg_view_window_select_previous_message (win) &&
+           !modest_msg_view_window_select_next_message (win)) {
                gboolean ret_value;
                g_signal_emit_by_name (G_OBJECT (win), "delete-event", NULL,
&ret_value);
        }
Comment 2 Aniello Del Sorbo (reporter) 2011-01-31 23:55:45 UTC
Created an attachment (id=3275) [details]
Patch to allow changing which message to show after deleting/moving a message
from the VIEW window

This patch allows for configuring which message to show after deleting or
moving a message from the View window.
By default the current behavior is preferred (showing the "next" message).
If the config key:

/apps/modest/widgets/modest-msg-view-window/after_delete_direction_show_next

is set to TRUE, then the behavior is swapped and the "previous" message is then
shown.
To specify the "direction" for the "move" command, the following key is to be
used:

/apps/modest/widgets/modest-msg-view-window/after_move_direction_show_next

This is useful when reading new e-mail starting from the oldest: deleting a
message will show the previous message (i.e. the next unread message).

PS: I couldn't find better names for the keys. I'm happy to change them.
Comment 3 Aniello Del Sorbo (reporter) 2011-02-01 01:44:47 UTC
Created an attachment (id=3276) [details]
Full patch to allow changing which message to show after deleting/moving a
message from the VIEW window. Includes UI to change setting in Settings.

Patch to allow changing which message to show after deleting/moving a message
from the VIEW window. Includes UI to change setting in Menu->Settings along
with en_GB translation.
Comment 4 naikel 2011-02-07 17:42:08 UTC
Well, Aniello patch works pretty well as it is  though I would change a couple
of things. First, I would put the gconf attribute in /apps/modest since the
widgets branch is supposed to have only UI attributes for each window, and this
is not an UI attribute.  And second, maybe set a single configuration for both
move and delete, instead of one for each.

Now, my humble opinion is that we shouldn't make this a configurable
enhancement and treat it like a bug, because it is a bug.  No other email
client in this planet shows the older mail when deleting or moving mails.  They
all show the newer mail, and no email client in this universe has a
configuration like this.

So I wonder what people think about it.  The first patch proposed in comment #1
is the right one I think.
Comment 5 Mohammad Abu-Garbeyyeh 2011-02-11 20:38:23 UTC
This has been fixed for the Community SSU Updates in package
modest 3.90.7-5
which is part of the internal build version
20.2010.36-2maemo11
(Note: 20.2010.36-2 is the latest official Nokia version.
The number after it indicates the Community SSU release version.)

A future public Community SSU update released within this week will include the
fix.
Please verify that this new version fixes the bug by marking this bug report as
VERIFIED after the public update has been released and if you have some time.

Commit:
http://gitorious.org/community-ssu/modest/commit/87d524d81f773575f04411d17aed8cee762c625a

For more information about Community SSU please see
http://wiki.maemo.org/Community_SSU
Comment 6 Andrew Flegg maemo.org 2011-02-17 10:38:53 UTC
Which is the patch which has been applied? Is it an option or not?
Comment 7 Aniello Del Sorbo (reporter) 2011-02-17 16:03:55 UTC
Hi Andrew,

from the commit in comment #5, it seems there is not option.
The direction was just inverted.