30560 - right-clicking a mail message/folder should not display it

Status: VERIFIED FIXED

(SeaMonkey :: MailNews: Message Display, defect, P2)

Description

[Richard Zach]

Right clicking in the folder pane selects the folder and displays the context
menu.  Right clicking in the thread pane does *not* select the message and
displays a context menu with all selections greyed out.

These should work the same, I'm not sure which way though.  In the browser,
right clicking on something does not result in a left-click effect, so probably
a right click in the folder pane should also not select the folder.  However,
I'd like the options to be there, e.g., I'd like to be able to right click on a
newsgroup and select "Unsubscribe from newsgroup" *without* opening the
newsgroup first.  On the other hand, it might be confusing in the thread pane if
you have one message selected and then right click on another message.  Right
now, all the actions you can select from the context menu apply to the selected
message, not the message you right-clicked on.

Linux build 2000.03.04.09

Comment 1

[Phil Peterson]

Reassign to putterman. Do we have any hope of getting the right context menu 
without selecting the tree row? We didn't in 4.x...

Comment 2

[scottputterman]

moving to future milestone.

Comment 3

[Michael Witbrock]

Right clicking on the folder name definitely should not open the folder.  
For me, at least, the thing that I use right click for on mail folders is to
create a new subfolder before filing a message. It is a source of intense
irritation to have the folder containing the message I'm trying to file closed,
and replaced by the folder I'm trying to create the subfolder of.  This is my #1
most annoying Mozilla bug.

(A context menu on individual messages, "file into", with the ability to select
a folder by typing its prefix (and the ability to create the folder if needed)
-- as provided by the otherwise revolting "Lotus Notes", would be even more
helpful. I spend about 50% of all the time I'm using Mozmail filing messages, so
facilitating this would be great.

Comment 4

[Calum Mackay]

I think that right-clicking anything, in any app, should never have
any action; it should merely bring up a context-sensitive menu.

Still, what do I know? :)

Comment 5

[R.K.Aa.]

*** Bug 80898 has been marked as a duplicate of this bug. ***

Comment 6

[R.K.Aa.]

*** Bug 83011 has been marked as a duplicate of this bug. ***

Comment 7

[R.K.Aa.]

*** Bug 101188 has been marked as a duplicate of this bug. ***

Comment 8

[Niko Pavlicek]

this is os -> all! I have the same problems on win98. could someone with the
necessary rights do this change?

Comment 9

[R.K.Aa.]

*** Bug 102512 has been marked as a duplicate of this bug. ***

Comment 10

[scottputterman]

reassigning to sspitzer.  

Comment 11

[Kai Engert [:KaiE:]]

Changing my e-mail address.

Comment 12

[Kai Engert [:KaiE:]]

Removing old e-mail address.

Comment 13

[scottputterman]

Fixing this would hide 101023 and is most likely the experience the user was
hoping for.

Comment 14

[scottputterman]

reassigning to ssu.

Comment 15

[R.K.Aa.]

*** Bug 108188 has been marked as a duplicate of this bug. ***

Comment 16

[Jesse Ruderman]

*** Bug 108183 has been marked as a duplicate of this bug. ***

Comment 17

[Jesse Ruderman]

*** Bug 104808 has been marked as a duplicate of this bug. ***

Comment 18

[Jesse Ruderman]

*** Bug 88119 has been marked as a duplicate of this bug. ***

Comment 19

[Jesse Ruderman]

See also bug 106687, "Context menu on outliner should not change selection?".

From bug 106687: Outlook Express's tree view appears to temporarily change the
selection during the context menu event, restoring it afterwards.

Comment 20

[Ronald van Kuijk]

Is this related to bug#30878 ? If I do a search on 'click right' in bugzilla I
see so many 'right click should'nt do this or that' that could simply be caused
by bug #30878

Comment 21

[Jesse Ruderman]

I think 30878 was about the appearance of buttons when you right-click on 
them.  (Some buttons used to appear depressed as you right-clicked on them.)

Comment 22

[Boris Zbarsky [:bzbarsky]]

*** Bug 95029 has been marked as a duplicate of this bug. ***

Comment 23

[Boris Zbarsky [:bzbarsky]]

*** Bug 113458 has been marked as a duplicate of this bug. ***

Comment 24

[Boris Zbarsky [:bzbarsky]]

*** Bug 115970 has been marked as a duplicate of this bug. ***

Comment 25

[scottputterman]

*** Bug 116319 has been marked as a duplicate of this bug. ***

Comment 26

[jglick]

Attachment: OE example

The Outlook Express, Folder Pane --> Thread Pane behavior is nice. Context menu
on a folder. Folder gets "selection" appearance (blue background) but messages
not displayed in Message Pane for that folder. Folder that was originally
selected has dotted line selection around it and its messages are still being
displayed in Message Pane. 

It would be nice if we could do this for the Folder Pane-->Thread Pane and
Thread Pane-->Message Preview Pane.

Comment 27

[Mitchell Stoltz (not reading bugmail)]

*** Bug 119674 has been marked as a duplicate of this bug. ***

Comment 28

[R.K.Aa.]

*** Bug 88028 has been marked as a duplicate of this bug. ***

Comment 29

[Sean Su]

Jennifer, I got all the attributes you listed working:

 * right-mouse clicking on a folder in the folder pane hilights the folder,
   but does not load the folder contents in the thread pane.
 * the context menu that appears for the folder is relative to the hilighted
   folder.
 * right-mouse clicking on a message in the thread pane hilights the message,
   but does not load the message in the message pane.
 * the context menu that appears for the message is relative to the hilighted
   message.

The only thing that it doesn't do right now is re-hilight the original
folder/message when the popup menu goes away.  I wanted to know exactly how you
want this 'right-mouse click' feature to behave.

Is there anything that I missed?

Comment 30

[jglick]

>The only thing that it doesn't do right now is re-hilight the original
>folder/message when the popup menu goes away.  I wanted to know exactly how you
>want this 'right-mouse click' feature to behave.

It would be great if you could do this as well. Since the context menu click 
didn't cause focus to change, the message headers of the original folder are 
still being displayed. So when the context menu is closed, focus should go 
back to the original folder. (same thing for message thread to message pane 
relationship).

Comment 31

[Ben Ruppel]

Sean: does the original folder (ie: the folder that will be re-selected once the
context menu is gone) retain any sort of mark or indication?  For instance, in
outlook 97 (yeah, I'm old school here), you right click on another folder and
the original folder becomes outlined with a dotted line while the temporarily
selected folder gets the selection color.

In any case, good work!  This will rock once it's checked in!

Comment 32

[Sean Su]

Ben, yes, it does leave an outline of what the previous (or still loaded)
folder/message was.

I just got the original folder/message to re-hilight when the context menu goes
away.  I'm making sure all the context menu commands still work on the hilighted
folder/message.  Almost done.

Comment 33

[Sean Su]

Attachment: patch v1.0

initial patch.	I'm sure it needs some things cleaned up still, but here it is
(finally).

Comment 34

[Sean Su]

Attachment: patch v1.1

This new patch fixes a problem with the previous patch in that it now does not
load the folder that has been right-mouse clicked on (in the folder pane).  I
had accidentaly deleted the change that prevented this from the initial patch.

So the only change in this patch (as compared to the previous patch) is in
commandglue.js:

   function FolderPaneSelectionChange()
   {
       if(gTimelineEnabled) {
	 gTimelineService.startTimer("FolderLoading");
	 gTimelineService.enter("FolderLoading has Started");
       }
       var folderOutliner = GetFolderOutliner();
  +    var ci = folderOutliner.outlinerBoxObject.selection.currentIndex;
  +
  +    if (!folderOutliner.outlinerBoxObject.selection.isSelected(ci))
  +	 return;
  +
       if (folderOutliner.outlinerBoxObject.selection.count == 1)
       {
	   var startIndex = {};
	   var endIndex = {};

Comment 35

[[e-mail]]

Actually what I see on OE is that only the folder pane has the magic
right-click, the thread pane always makes the context-clicked item current.
Perhaps you can make right-click magic only if there is already a single selection?

Now onto the part of the patch I don't understand.

Why you need a currentSelectedIndex? I can't see what effect it has. Or is it
because of the places where it isn't set that it has an effect (tricky, that!).
When restoring the selection you can use the same trick that the
FolderPaneSelectionChange uses to see if the current index is selected.
I can't see why MsgSaveAsFile can't use GetFirstSelectedMessage.
In fact I can't see why MsgOpenNewWindowForMessage can't use it either.
MsgOpenNewWindowForFolder could also be using GetFirstSelectedMsgFolder.

Finally, this popup showing and hiding. You've exposed a performance bug here,
popupshowing and popuphiding events bubble up from submenus and so the thread
pane context menu has been filled in every time a submenu opens! Fortunately
there is a quick fixed of adding if (event.target == this) into the event
handlers avoiding the need for global variables.

Other than that, I really like this patch - I shall be trying it out!

Comment 36

[scottputterman]

this can land in 1.0

Comment 37

[[e-mail]]

Attachment: My take on the patch

I noticed that there might be a problem if you right-clicked a multiple
selection.
I also enabled the open in new window action for servers, because it now works.

I also simplified some of the code as per my previous comment.

Comment 38

[Blake Ross]

This is not necessarily the only place an outliner will ever want this behavior,
so I am confused to see no changes to outliner.xml...?

Comment 39

[[e-mail]]

Attachment: Revised my patch

I see what the problem was now. GetFirstSelectedMessage() actually returns the
current loaded message, instead of the first selected message. With the new
code this makes a difference! I have therefore revised my patch accordingly.

I have not changed outliner.xml for several reasons:
1) It might make the patch harder to review
2) Some windows e.g. History expect a single selection to be the current index
3) I know no reliable way to detect the use of a context menu

Comment 40

[[e-mail]]

I've spotted a new problem with any of these patches.
When you delete a message, its next message is automatically selected...
this should only apply when the current message is removed from the view.

Comment 41

[Sean Su]

Attachment: patch v1.2

This revised patch no longer requires a change to the outliner code (as Neil
had pointed out that it was no longer necessary) and also fixes the problem
when deleting a message in the thread pane via the right-mouse click.  It will
now simply re-select the message that contains the outlined border upon
deletion.

Comment 42

[[e-mail]]

Comment on attachment 71989 [details] [diff] [review]
patch v1.2

Possible problems:
you should return from FolderPaneSelectionChange before starting the timer.

both ThreadPaneOnClick and ThreadPaneOnMouseDown try to change the selection.

there's a comment on an unused global variable.

Possible enhancements:
allow open in new window for servers as well.

popupshowing for the threadPaneContext popup could do with the if test
(folderPaneContext currently doesn't need it because it doesn't have any
submenus).

the folder and thread pane mouse down functions only differ in the affected
outliner which is conveniently available in the "this" identifier.

MsgOpenNewWindowForMessage is only called in two places, once from
MsgOpenSelectedMessages and once from the context menu, the context menu could
call MsgOpenSelectedMessages instead, then inline
MsgOpenNewWindowForMessage into it.

Comment 43

[Sean Su]

Attachment: patch v1.3

> Possible problems:
> you should return from FolderPaneSelectionChange before starting the timer.

updated

> both ThreadPaneOnClick and ThreadPaneOnMouseDown try to change the selection.


this is necessary because both 'onclick' and 'onmousedown' events in the
outliner will change the selection as well.

> there's a comment on an unused global variable.

there is? where?

> Possible enhancements:
> allow open in new window for servers as well.

please file a new bug.

> popupshowing for the threadPaneContext popup could do with the if test

I'm trying to change the least amount of code as possible.  I didn't see
problems with the way it currently is.

> (folderPaneContext currently doesn't need it because it doesn't have any
> submenus).

I put the if() test there in case someone added submenus in the future.

> the folder and thread pane mouse down functions only differ in the affected
> outliner which is conveniently available in the "this" identifier.

updated to use one function now.

> MsgOpenNewWindowForMessage is only called in two places, once from
> MsgOpenSelectedMessages and once from the context menu, the context menu
could
> call MsgOpenSelectedMessages instead, then inline
> MsgOpenNewWindowForMessage into it.

This is not a fix that is required for this bug.  Besides, when I tried (as I
followed in your latest patch), the menu command no longer worked properly. 
I'm leaving it as is for now since it's not part of this bug.

Comment 44

[[e-mail]]

I take your point on all but two comments:

> there's a comment on an unused global variable.

Sorry, I forgot to name the variable: gOnThreadPanePopupShowingCount

> both ThreadPaneOnClick and ThreadPaneOnMouseDown try to change the selection.

Left-click and right-down try to change the selection, but not right-click.

Comment 45

[Sean Su]

>> there's a comment on an unused global variable.
>Sorry, I forgot to name the variable: gOnThreadPanePopupShowingCount

found it.  comment removed in my local tree.

>> both ThreadPaneOnClick and ThreadPaneOnMouseDown try to change the selection.
>Left-click and right-down try to change the selection, but not right-click.

It's actually on any button mouse down (left-mouse down or right-mouse down). 
In patch v1.3, I had already combined and renamed these two functions (as you
suggested) to the name you had in your patch (seemed appropriate):
OutlinerOnMouseDown

Is this the change you wanted for these functions in the patch?

Comment 46

[racham]

Comment on attachment 72148 [details] [diff] [review]
patch v1.3

Looks like the patch is doing the right thing on all right-clicks. For any
enhancements, please do file separate bugs and mention those bug numbers here.

r=bhuvan

Comment 47

[(not reading, please use [e-mail] instead)]

You're going to make jag very happy with this patch.  He's been asking for this
for a while.

Some comments:

1)

I'd add a local variable for outlinerBoxObj.selection.

It will make the code more readable, and it should be faster.

+// Function to change the highlighted row back to the row that is currently
+// outline/dotted without loading the contents of either rows.
+function RestoreSelectionWithoutContentLoad(outliner)
+{
+    var outlinerBoxObj = outliner.outlinerBoxObject;
+    var ci = outlinerBoxObj.selection.currentIndex;
+
+    if((!outlinerBoxObj.selection.isSelected(ci)) &&
+       (outlinerBoxObj.selection.currentIndex >= 0))
+    {
+        outlinerBoxObj.selection.selectEventsSuppressed = true;
+        outlinerBoxObj.selection.select(outlinerBoxObj.selection.currentIndex);
+        outlinerBoxObj.ensureRowIsVisible(outlinerBoxObj.selection.currentIndex);
+        outlinerBoxObj.selection.selectEventsSuppressed = false;
+    }
+}

2)

I'd add some local variables for outliner.outlinerBoxObject and
outliner.outlinerBoxObject.selection

It will make the code more readable, and it should be faster.

+// Function to change the highlighted row to where the mouse was clicked
+// without loading the contents of the selected row.
+// It will also keep the outline/dotted line in the original row.
+function ChangeSelectionWithoutContentLoad(event, outliner)
+{
+    var row = {};
+    var col = {};
+    var elt = {};
+    outliner.outlinerBoxObject.getCellAt(event.clientX, event.clientY, row,
col, elt);
+    if(row.value >= 0)
+    {
+        var saveCurrentIndex = outliner.outlinerBoxObject.selection.currentIndex;
+        outliner.outlinerBoxObject.selection.selectEventsSuppressed = true;
+        outliner.outlinerBoxObject.selection.select(row.value);
+        outliner.outlinerBoxObject.selection.currentIndex = saveCurrentIndex;
+        outliner.outlinerBoxObject.ensureRowIsVisible(row.value);
+        outliner.outlinerBoxObject.selection.selectEventsSuppressed = false;
+    }
+    event.preventBubble();
+}
+


3) 

make sure this code works with both "move to trash" and "imap delete model"
 
 function SetNextMessageAfterDelete()
 {
+  var outlinerBoxObj = GetThreadOutliner().outlinerBoxObject;
+
+  if (outlinerBoxObj.selection.isSelected(outlinerBoxObj.selection.currentIndex))
     gNextMessageViewIndexAfterDelete = gDBView.msgToSelectAfterDelete;
+  else
+    gNextMessageViewIndexAfterDelete = outlinerBoxObj.selection.currentIndex;
 }
 
4)  naving added code so that if you quick searched, and then clicked back on
the same folder, it would clear the quick search text and show you the whole
folder.  please test and make sure that still works.

5)  test well, these are non-trivial changes.  when testing, look for funcions
shared by the advanced message search dialog, or the stand alone msg window,
make sure everything still works.

sr=sspitzer once you address these issues.  Before checking in, let's get a
r=neil on this patch, too.

Comment 48

[jag (Peter Annema)]

*jumping up and down*

Comment 49

[Sean Su]

Attachment: patch v1.4

This new patch addresses Seth's concerns:
 1) fixed
 2) fixed
 3) verified, but in the process, found another slight problem:
      If the currentIndex (row with the outline/dotte line) is > the currently
      selected index (row with the highlight), then when a Delete or Move is
      performed on the selected index, the highlight is restored on the row
      after the currentIndex.

      This is because after the Delete or Move command, there the number of
rows
      is now (row - 1) while the currentIndex hasn't been updated to reflect
      this condition.

      This would be one place where I could use a new attribute in the outliner

      to figure out which row is currently selected as compared to currently
      outlined.

      Since changing the outliner would be a royal pain, I simply added a
global
      var used only by the thread pane to keep track of which row is selected.
      This fixes the problem.

 4) verified that it does still work, with one exception:
      1) select a folder
      2) in the quick search textbox, enter a string to search for
      3) right-mouse click in the folder pane on a different folder other than
	 the one already selected.

    In this instance, the thread pane does not get reset nor does the quick
    search textbox.  This might be because the right-mouse clicked folder has
    not been loaded.

    I'm assuming this particular instance is okay because when a left-mouse
    click is performed on any folder in the folder pane, the thread pane gets
    loaded and the quick search textbox is cleared.

 5) I found out that the Xul elements for both the threadpane and the advanced
search dialog (the bottom list area where the results are shown) use the same
Id, 
'threadOutliner'.  I fixed my code to not allow this special right-mouse click
feature to work in the advanced search dialog by checking it's parentNode id
value.

Neil, could you review this new patch?

Comment 50

[[e-mail]]

Comment on attachment 73077 [details] [diff] [review]
patch v1.4

Sorry, but I still don't see why you need to change the ThreadPaneOnClick
function, the code seems to work fine as for the folder pane, i.e. only
trapping right mouse down.

Also, outliner.getAttribute("id") can be replaced by outliner.id which I assume
is preferred.

Comment 51

[Sean Su]

Attachment: patch v1.5

You are correct, the change to ThreadPaneOnClick() is not needed.  It was
needed at one point, but the more recent checkins to the outliner might have
altered its behavior.  Change to ThreadPaneOnClick() backed out.

Also updated patch to use outliner.id instead of outliner.getAttribute('id').

Comment 52

[[e-mail]]

Comment on attachment 73375 [details] [diff] [review]
patch v1.5

r=[e-mail]

Comment 53

[[e-mail]]

Comment on attachment 73375 [details] [diff] [review]
patch v1.5

That r= was given while testing MailNews offline and I'm not convinced that I
was getting a delete completed for an offline IMAP delete, so I haven't after
all been testing the code to set the next message after a delete. I am worried
that it won't actually work with the IMAP delete model.

Comment 54

[(not reading, please use [e-mail] instead)]

neil writes:

>I am worried that it won't actually work with the IMAP delete model.

ssu, can you confirm that it works?

Comment 55

[Sean Su]

I had tested the IMAP delete before and saw that it did work.  I'm updating my
tree and will veify it again.

Comment 56

[(not reading, please use [e-mail] instead)]

Comment on attachment 73375 [details] [diff] [review]
patch v1.5

looks good.  make sure to test well, especially different delete models.

two minor suggestions, then sr=sspitzer

1)

 function FolderPaneSelectionChange()
 {
+    var folderOutliner = GetFolderOutliner();
+    var ci = folderOutliner.outlinerBoxObject.selection.currentIndex;
+
+    if (!folderOutliner.outlinerBoxObject.selection.isSelected(ci))
+      return;

A suggestion:

+    var folderOutliner = GetFolderOutliner();
+    var folderSelection = folderOutliner.outlinerBoxObject.selection;
+    if (!folderSelection.isSelected(folderSelection.currentIndex))
+      return;

2)

+  var outlinerBoxObj = GetThreadOutliner().outlinerBoxObject;
+
+  if
(outlinerBoxObj.selection.isSelected(outlinerBoxObj.selection.currentIndex))
+    outlinerBoxObj.view.selectionChanged();

I think a 

var outlinerSelection = outlinerBoxObj.selection;

might help here, too.

Comment 57

[Sean Su]

Attachment: patch v1.6

This latest patch addresses Seth's latest comments and also the following (as
compared 

to patch v1.5):
 * fixes a problem with the highlight not being cleared from a row in the case
   of:
     initial folder being loaded, but no messages selected.  Right-mouse
clicking
     on a message highlights the row, but does not clear the highlight when the

     context menu goes away.

 * fixes problem where there are multiple rows selected it would clear all
   selections and select only the row that was right-mouse clicked on.	The fix

   leaves the original highlighted rows highlighted and still show the context
   menu.  Deletion still works properly here via the context menu.

 * fixes the problem where in the thread pane, the context menu would not
enable
   the delete menu item in the case of:
     initial selection of a folder where there are no messages selected or any
     loaded in the message pane.

IMAP Delete was also verified to be worked properly.

Neil, could I get another r=?  I'm hoping this is the final patch *fingers
crossed*

Comment 58

[[e-mail]]

Comment on attachment 73614 [details] [diff] [review]
patch v1.6

I don't like the searchResultListBox test, and not just because you've missed
the ; after the return. Can you arrange not to add the event listener in the
search dialog by moving the test to ThreadPaneOnLoad instead?

I can't see the point of SetupDeleteMenuItem, it ignores numSelected and
forceHide is always true. Mind you, I can't reproduce the problem with a
right-click in a newly loaded thread pane either!

Also, now you've added the folderSelection variable to
FolderPaneSelectionChange is it worth touching the following lines as well?

Something that I hadn't noticed earlier, I'm worried that scrolling back to the
previous selection might be confusing. i.e. with a large inbox, select a
message, scroll to the other end, right-click, whatever, the outliner would
scroll back :-(

Good to see the right-click on existing multiple selection fixed (as per
attachment 71213 [details] [diff] [review] :-).

Comment 59

[Sean Su]

Attachment: patch v1.7

> I don't like the searchResultListBox test, and not just because you've
> missed the ; after the return. Can you arrange not to add the event
> listener in the search dialog by moving the test to ThreadPaneOnLoad
> instead?

added the missing ';'

searchResultListBox test has been moved from
ChangeSelectionWithoutContentLoad() to ThreadPaneOnLoad().  This does look
better.  It no longer adds OutlinerOnMouseDown() to the mousedown event
listener.

> I can't see the point of SetupDeleteMenuItem, it ignores numSelected
> and forceHide is always true. Mind you, I can't reproduce the problem
> with a right-click in a newly loaded thread pane either!

SetupDeleteMenuItem() now uses numSelected appropriately.

> Also, now you've added the folderSelection variable to
> FolderPaneSelectionChange is it worth touching the following lines as
> well?

I've gone thru the function and used 'folderSelection' where
'folderOutliner.outlinerBoxObject.selection' was used.	I'm assuming that's
what you meant.

> Something that I hadn't noticed earlier, I'm worried that scrolling
> back to the previous selection might be confusing. i.e. with a large
> inbox, select a message, scroll to the other end, right-click,
> whatever, the outliner would scroll back :-(

I thought about this one too.  This was actually the default behavior.	I do
see your point that if the user did as you did in your test case, it would be
rather annoying to have to scroll back down to right-click on other messages to
work on.  I'm sorta on the fence with this one.  Perhaps once this bug is
fixed, and it becomes rather obvious that it is indeed a usability issue, we
should be able to easily fix it.

However, if Jglick has an opinion on what the default behavior should be, I'm
all ears.

This new patch addresses Neil's comments above and the following:
 * On a newly loaded folder in an IMAP account (when there's no
   message loaded in the message pane), righ-clicking on a message in
   the thread pane and selecting the 'Delete Message' menu item would
   successfully delete the message, but the outliner view would not
   update.

   This would work fine in a POP account.

 * Assertions when right-clicking on messages in the thread pane on
   a newly loaded folder (when there's no message loaded in the message
   pane) in either IMAP or POP accounts.

seeking r= again.

Comment 60

[[e-mail]]

Comment on attachment 73724 [details] [diff] [review]
patch v1.7

r=[e-mail]

Comment 61

[jglick]

> Something that I hadn't noticed earlier, I'm worried that scrolling
>> back to the previous selection might be confusing. i.e. with a large
>> inbox, select a message, scroll to the other end, right-click,
>> whatever, the outliner would scroll back :-(

>This was actually the default behavior.  I do
>see your point that if the user did as you did in your test case, it would be
>rather annoying to have to scroll back down to right-click on other messages to
>work on.  I'm sorta on the fence with this one.  Perhaps once this bug is
>fixed, and it becomes rather obvious that it is indeed a usability issue, we
>should be able to easily fix it.

Ideally, it would be nice if the user highlighted a message, scrolled down, 
right-clicked on a different message, dismissed context menu, originally 
highlighed messages remains highlighted but we don't scroll back up, but remain 
at the current position the user scrolled down to.

Comment 62

[(not reading, please use [e-mail] instead)]

Comment on attachment 73724 [details] [diff] [review]
patch v1.7

sr=sspitzer

I know ssu has really tested this one.	sean, can you sit down with Olga and
show her all the test cases you went through, or enumerate those test cases in
the bug, for her benefit?

Comment 63

[Asa Dotzler [:asa]]

Comment on attachment 73724 [details] [diff] [review]
patch v1.7

a=asa (on behalf of drivers) for checkin to the 1.0 trunk

Comment 64

[Sean Su]

patch checked in to trunk only.

Olga, here is a quick summary of the tests that I did.  I'll stop by and talk to
you in person to go over the details of my tests to make sure it is understood.

POP, IMAP, and news accounts in both regular 3pane and alternate 3 pane window
modes:
  Folder Pane:

    *  made sure all context menu items worked on right-clicked folder
    *  made sure all context menu items still worked on left-clicked
      folder


  Thread Pane:

    * Tested before any other messages were selected and after at
      least one message was selected in the thread pane:

        * made sure all context menu items worked on right-clicked
          message
        * made sure all context menu items still worked on
          left-clicked message
        * made sure all context menu items worked on right-click
          on an already highlighted message in a multiple
          message/row selection.

Make sure that the advanced search dialog still works correctly since it uses
the same outliner id the the thread pane.

Comment 65

[Sean Su]

by the way, I forgot to mention that I implemented the non-autoscroll back that
neil mentioned and jglick had concurred with.  It was a simple deletion of one
line from my patch:

  outlinerBoxObj.ensureRowIsVisible(outlinerSelection.currentIndex);

from the RestoreSelectionWithoutContentLoad() function in mailContextMenus.js.

Comment 66

[[e-mail]]

r=[e-mail] on that deletion :-)

Comment 67

[Calum Mackay]

I'm currently using 2002031410 on Solaris SPARC built here from local source,
and am seeing a few anomalies:

o Right-clicking on a folder does now correctly bring up a context menu without
displaying the folder, which is good. However, when I use the context menu to
bring up the folder in a new window (the first option "Open in New Mail Window")
the new window appears but it doesn't have any folder loaded in it.

This worked up until recently (albeit with the folder also being loaded in the
first window - this bug) and I'm wondering if this fix has possibly broken it?

o If a folder is manually loaded into the new (currently empty) mail window,
message headers are not displayed. The message header panel is simply missing,
although it is present in the first mail window.

Again this worked very recently, breaking around when *this* bug was fixed.

These might be unrelated bugs of course...

Comment 68

[[e-mail]]

Filed bug 130846 on that issue.

Comment 69

[Aleksey Nogin]

Another issue:
1) Left-click on folder
2) Scroll down the header pane, right-click on the last message
3) Use the context menu to move it to another folder.

Actual: header pane displays first "n-1" headers (where n is how many it normaly
displays) in the bottom n-1 rows with the top row completely empty.

Expected: n headers displayed as usual.

Comment 70

[Sean Su]

looks like delete and the move commands are still calling the
ensureRowIsVisible() function.   I've filed bug 130982

Comment 71

[(not reading, please use [e-mail] instead)]

note to jglick and ssu,

see IE's history sidebar for some similar UI issues.

They allow one item to be selected, and then allow right click of another item 
(context menu delete) and then they reselect the originally selected item.

just a datapoint.

Comment 72

[:deanis74]

*** Bug 131210 has been marked as a duplicate of this bug. ***

Comment 73

[Michael Klose]

I downloaded a build on my Win2K computer at work Friday (around 3 PM European
time, I don't know which build as I am not at work right now), and the following
happens when I delete a mail by right clicking a mail further down than selected
for viewing:

As soon as I delete the mail, it is deleted, but in the message list at the top
I (where the first message info should be) get a little envelope symbol and the
message listings are not quite right. As soon as I click on a different message,
everything is OK again.

I would provide a screen shot, and I will, but I was rather busy and won't get
back there before Tuesday.

Still, this is one of the best features to be included into Mozilla Mail for a
very long time. Thanks to everyone involved.

Comment 74

[Andreas Premstaller]

Using 2002031803 in W2K, and IMAP with "mark as deleted":
Deleting a message from its context menu, the following happens:
1. Right click on message, context menu pops up, message is not displayed (nice)
2. Select Delete Message (or Move Message)
3. Message is marked deleted
4. Message body is loaded in the message pane

#4 should not happen. 

Should I reopen this bug or file a new one?

Comment 75

[[e-mail]]

Andreas, is bug 130982 relevant?

Comment 76

[Calum Mackay]

I couldn't reproduce Andreas's problem in comment 74; I'm on 2002031814 on
Solaris/SPARC. I see:

1. Right click on message, context menu pops up, message is not displayed (nice)
2. Select Delete Message (or Move Message)
3. Message is marked deleted
4. Message body is NOT loaded in the message pane, as expected. The loaded msg
in the pane doesn't change.

Comment 77

[Andreas Premstaller]

Neil, re: comment 75: I do not think that bug 130982 is relevant to my problem
(or at least, the patch does not solve it), since a patch for that bug was
checked in 2002-03-15, and my Windows nightly is 2002-03-18-03, and should this
already have the patch incorporated.

After Calum's comment 76, I tried build 2002031913 on Linux, the behavior is
different from the Windows build for me (same IMAP mail account):
* If I access a message folder with no message selected and displayed yet, and
delete any message via context menu (without first selecting it with left
click), then after deletion (mark as deleted) the message is displayed in the
message pane. 
* If a message is already selected and displayed, and the message I delete via
content menu is below the actually selected one in the thread pane, then the
deleted message is not displayed (nice!)
* If a message is already selected and displayed, and the message I delete via
content menu is above the actually selected one in the thread pane, then after
deletion the display changes to the message above the one selected before
deletion. (Assuming the message was removed and not just marked as deleted, this
makes sense, since then the marked message wanders up by one from position n to
n-1, but not with the IMAP marked as deleted model).  

Comment 78

[[e-mail]]

I can duplicate that last issue of comment 77.

Comment 79

[Andreas Premstaller]

I tested again on Windows 2000 with build 2002032003, the behavior for me is
exactly the same as in the Linux build and as described in the second part of
comment 77. (no difference between Windows and Linux, I did not test all cases
on Windows the first time)

Comment 80

[[e-mail]]

What I see when opening a folder, scrolling, and deleting a message (IMAP delete
model) is that the folder scrolls back to the top :-(

Comment 81

[[e-mail]]

Wait, do I have the fix for 130982? I'd better double-check. Sorry for the SPAM.

Comment 82

[Kai Engert [:KaiE:]]

I filed bug 134952, which might to be a missing issue from this implementation.

Comment 83

[Olga]

Verified on 04/05/2002 trunk build - Win2K, Linux, Mac OSX (Control+click).
Thanks for neat design and implementation!
Steps - all results as expected!
1. Initial state: selected Inbox, IMAP account, 3-pane.
2. Right click on Trash folder. Got expected context menu including Empty Trash.
3. Press ESC - context menu disappear and previously selected Inbox gets bright
hightlighted again. No changes in the Thread pane or Message pane.
4. Right click on any other message from Thread pane. Got related context menu.
ESC - no changes with previously selected, highlighted, and displayed message.
5. Also used menu items from context menus by activating them by left or right
click (as Sean suggested).  Works!
6. I did the same and different tests with Newsgroup and POP accounts.

Comment 84

[Ben Ruppel]

Anyone else see some erroneous results with this?  I've had the right-click and
"mark newsgroups read" function mark the already selected newsgroup read.  I've
also right clicked on a message, deleted it, and had a third message (neither
the originally selected or the right-clicked on one) deleted.

Comment 85

[[e-mail]]

Marking a newsgroup read looks like it won't work because that function
considers the current folder, not the selected folder. But I've never had any
problem deleting messages, except that the wrong message is sometimes selected
after delete, or that the thread pane gets scrolled to the top.

Comment 86

[Ben Ruppel]

I can reproduce it on my win32 box now.  Select a message.  Right click on and
choose to delete the message two messages up from it.  Instead of deleting that
message, the message in between the selected and the right-clicked message is
deleted.  I'm using an april 16 build.

Comment 87

[[e-mail] (gone - use [e-mail] instead)]

*** Bug 150631 has been marked as a duplicate of this bug. ***