Closed Bug 187673 Opened 22 years ago Closed 22 years ago

Mail title is not consistent with the mail message

Categories

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

x86
All
defect

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: leon.zhang, Assigned: sspitzer)

References

Details

Attachments

(1 file, 5 obsolete files)

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
Priority: -- → P2
Priority: P2 → P1
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.
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.
after analyze more carafully, I think we can have a more simple solution.
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.
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.
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?
This is major, not critical (no crash, hang or dataloss).
Severity: critical → major
Comment on attachment 110655 [details] [diff] [review] set the title of main window to the opened message window r=dmose
Attachment #110655 - Flags: review+
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?
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?
Severity: major → critical
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?
Severity: critical → major
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 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.
Attachment #110655 - Flags: superreview?(sspitzer)
up to now, reply issues still exist. I want to do with it in another bug.
I filed a new bug http://bugzilla.mozilla.org/show_bug.cgi?id=187852 for the reply issues.
I still think we should clone the dbview when we open a message window to display the email.
Attached patch new patch(can resolve reply issues) (obsolete) (deleted) — Splinter Review
Attachment #110655 - Attachment is obsolete: true
Attachment #110745 - Attachment is obsolete: true
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.
Attachment #110746 - Flags: superreview?(sspitzer)
Attachment #110746 - Flags: review?(dmose)
Blocks: 187852
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.
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
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.
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.
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.
Attached patch patch1.1(clone when Dbview changed) (obsolete) (deleted) — Splinter Review
Attachment #110746 - Attachment is obsolete: true
Attachment #110856 - Flags: review?(dmose)
Severity: major → critical
Attachment #110856 - Attachment description: patch1.1(clone when Dbciew changed) → patch1.1(clone when Dbview changed)
Attachment #110655 - Flags: superreview?(sspitzer)
Attachment #110746 - Flags: superreview?(sspitzer)
Attachment #110746 - Flags: review?(dmose)
Keywords: patch
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.
Attachment #110856 - Flags: review?(dmose)
Attached patch patch based on leon's work. (deleted) — Splinter Review
Attachment #110856 - Attachment is obsolete: true
oops, I meant leon, not kyle. thanks for the excellent bug report, leon. can you review the patch?
Status: NEW → ASSIGNED
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.
Attachment #111075 - Attachment description: patch based on kyle's work. → patch based on leon's work.
fixed. the patch has r/sr=bienvenu
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
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.
*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.
Attached patch make clone Dbview more effective (obsolete) (deleted) — Splinter Review
seth, I will file another bug to this optimization.
btw, currenly, we can reuse the opened messgae window, so we also use the nonchnaged Dbview.
The about optimization is 188459
Attachment #111126 - Attachment is obsolete: true
thanks for logging the spin off bug. we'll take the discussion of the optimization to that bug.
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.
Status: RESOLVED → VERIFIED
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: