187673 - Mail title is not consistent with the mail message

Status: VERIFIED FIXED

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

Description

[leon.zhang]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.0.1) Gecko/20020823 Netscape/7.0
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.0.1) Gecko/20020823 Netscape/7.0

Mail title is not consistent with the mail message

Reproducible: Always

Steps to Reproduce:
Steps to Repro:
1.Start mozilla Mail.
2.Quick search by inputing a word in field"Subject or Sender contains:".
3.Double click a message.
4.Click Clear at the right of the quick search field to show all the messages.
5.Double click another message(not in the previous list resulted from search).
Actual Results:  
The title of the new opened message is the old one.

Expected Results:  
The title of the new opened message is the current selected messgae

Comment 1

[leon.zhang]

The bug is caused by:

when user input a string in search file, will create a  new msgDbview, and old
one is save in PreQuickSearchView.
when user double click, and open a selected message, will cloneDBView for the
new open message window.

After user "clear" search fileld, the old DBView(PreQuickSearchView)  will be
the current DbView. but the new open window's DBView is still the result of search.

So, If you open a another message(not in the result of search),will cause the
display error,although the body of the messgae is loaded correctly.

Comment 2

[leon.zhang]

we can do:
1) open a new window for every selected messgae when double click a messgae.
2) synchronize the DBview of the opened message window.

1) is quick,a nd is the old dislpayling pattern of selected messgaes.

2) need more modification to source codes. 

Comment 3

[leon.zhang]

after analyze more carafully, I think we can have a more simple solution.

Comment 4

[leon.zhang]

Attachment: set the title of main window to the opened message window

Comment 5

[leon.zhang]

Currently, mozilla can load the body of message correctly in open message
window, although the necessary Dbview is not cloned.

Now,only setting the correct title for the message window can resolve this bug.

Comment 6

[leon.zhang]

If we open a message in a message window many times, we should not clone a
DBview each time. 

currently, "windowID.gDBView.loadMessageByMsgKey(msgHdr.messageKey);" can load
each  selcted message correctly message window, so clone is not necessary.

above thoughts can explain current patch. 

Comment 7

[Andreas Kunz]

Bug 116024 sounds very similar.
But if you consider it a dup, I'd suggest duping that other one against this
one, because that other bug is quite dead and all work has been done here.

Is this really a 'critical' bug?

Comment 8

[Oliver Klee]

This is major, not critical (no crash, hang or dataloss).

Comment 9

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 110655 [details] [diff] [review]
set the title of main window to the opened message window

r=dmose

Comment 10

[Jay Yan]

Just a reminder:

Actually, the misbehavior is not only wrong title, if you reply that email with
wrong title, the quoted content is also wrong in the newly created email.

Not sure whether this patch can fix both problems?

Comment 11

[leon.zhang]

jay, Can we filed another bug for the misbehavior related to reply ?
I think this patch can only reslove incorrect display of mail window title.
I will file another bug for the second misbehavior, OK?

Comment 12

[leon.zhang]

jay, Can we file another bug for the misbehavior related to reply ?
I think this patch can only reslove incorrect display of mail window title.
I will file another bug for the second misbehavior, OK?

Comment 13

[Jay Yan]

sure, if seperated bugs are more helpful to solve all the problems.

BTW, I am curoius, can we change back to the former working mode of opening
email, all related problems are regressions of changing working mode of opening
email, what is the benifit of this mode changing? 

Comment 14

[Henry Jia]

Comment on attachment 110655 [details] [diff] [review]
set the title of main window to the opened message window

Seth, please take a look at this patch?

Leon, do you think this patch can't solve the related reply issue? Just make
sure.

Comment 15

[leon.zhang]

up to now, reply issues still exist.
I want to do with it in another bug.

Comment 16

[leon.zhang]

I filed a new bug http://bugzilla.mozilla.org/show_bug.cgi?id=187852 for the
reply issues.

Comment 17

[Antonio.Xu]

I still think we should clone the dbview when we open a message window to
display the email.

Comment 18

[leon.zhang]

Attachment: new patch(can resolve reply issues)

Comment 19

[leon.zhang]

Attachment: patch1.0(cloned a new Dbview for another message in opened messgae window)

Comment 20

[leon.zhang]

After cloned a new Dbview, memebers of opened window have been synchronized too.

the detailed process is below:

1)mailWindowOverlay.js: MsgOpenExistingWindowForMessage : -->
2)windowID.gDBView.loadMessageByMsgKey(msgHdr.messageKey); ......>
3)nsMsgDBView::LoadMessageByMsgKey(nsMsgKey aMsgKey) 
4)nsMsgDBView::UpdateDisplayMessage(nsMsgKey aMsgKey)
5)mCommandUpdater->DisplayMessageChanged(m_folder, subject, keywords).....>
6)messageWindow.js:displayMessageChanged -->
   (1) setTitleFromFolder(aFolder, aSubject);
   (2) gCurrentMessageUri = gDBView.URIForFirstSelectedMessage;


because process above, current patch can finish the jobs of previous patches.  

Comment 21

[Dan Mosedale (:dmosedale, :dmose)]

Is this really fixing the right bug?  I would expect double-clicking two
different messages to open two separate message windows, not open one message
window and reuse it.

Comment 22

[leon.zhang]

There is a switch in preference(new trunk), so that you can select your mode:
using the exist  window or a new one. Default mode is using exist widnow when
double click the a message.

Edit --> preference --> Mail/newsgroup --> windows

Comment 23

[Dan Mosedale (:dmosedale, :dmose)]

OK, that makes sense.  Next question: is cloning the DB view on every message
the best strategy?  It seems like it might be possible to only clone for already
open windows if and when the dbview itself changes (or is cleared).  This would
avoid unnecessary cloning.  Whether this is actually practical (and whether
cloning a dbview is expensive enough to make this worth the effort), I don't
know, but I imagine sspitzer or bienvenu would.

Comment 24

[leon.zhang]

I have an idea from demose's comments:
we can saved the dbview of main window into current opened message window, so
when we find the saved value is different from the current vlaue of main window,
we can clone one, else cancel.

I will give a patch about this thought.

Comment 25

[Antonio.Xu]

I have another idea about this issue, you can set a global variable in
mailWindowOverlay.js named gdbviewchanged, when thread pane's dbview has been
changed, we could set this variable is true. then if user want to open a message
in the messagewindow, we could judge the global variable whether we should clone
the dbview or not.

Comment 26

[leon.zhang]

Attachment: patch1.1(clone when Dbview changed)

Comment 27

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

Comment on attachment 110856 [details] [diff] [review]
patch1.1(clone when Dbview changed)

kyle, thanks for the patch.

your on the right track, but I think the right fix is something even simpler.

I'll attach a new patch, based on yours.

something to keep in mind is that CreateView(gDBView) will clone the view. 

my patch will make it so we always call CreateView(), even if the folder uri is
the same.

with QS, and mail views, we can't assume the current view in the stand alone
msg window is valid.

Comment 28

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

Attachment: patch based on leon's work.

Comment 29

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

oops, I meant leon, not kyle.

thanks for the excellent bug report, leon.

can you review the patch?

Comment 30

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

giving a heads up to laurel:  

in addition to the mail title, I bet there are other bad things (like view
navigation) that would be broken because we were using the wrong view.

Comment 31

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

fixed.  the patch has r/sr=bienvenu

Comment 32

[leon.zhang]

seth,
I still think that we can prevent clone/create Dbview  when we still in the same
folder. I think this will be more effective to save current folder status.

If we have many messages which is not in previous search results, cloning for
any selected message will cost a lot.

dmose's comments #23 is the same thought.

Comment 33

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

*creating* a view on every double click can be expensive for large views.
but we are cloning, and I think cloning is reasonable.

you are right, it is possible to optimize further.
I'm not convince it is worth it.

feel free to spin up a new bug about the optimization.
you'll need to figure out when we can (and can't) re-use the view.

in addition to QS, we need to worry about mail views.

at this point, I chose to go with correctness and code simplicity, over
optimization.

Comment 34

[leon.zhang]

Attachment: make clone Dbview more effective

Comment 35

[leon.zhang]

seth, 
I will file another bug to this optimization.

Comment 36

[leon.zhang]

btw,
currenly, we can reuse the opened messgae window, so we also use the nonchnaged
Dbview.

Comment 37

[leon.zhang]

The about optimization is 188459

Comment 38

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

thanks for logging the spin off bug.  we'll take the discussion of the
optimization to that bug.

Comment 39

[laurel]

OK using 2003-01-13 commercial trunk: win98, linux rh8.0, mac OS 10.2
Tested with both reuse window or open in new message window.