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)
Tracking
(Not tracked)
VERIFIED
FIXED
People
(Reporter: leon.zhang, Assigned: sspitzer)
References
Details
Attachments
(1 file, 5 obsolete files)
(deleted),
patch
|
Details | Diff | Splinter Review |
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
Reporter | ||
Updated•22 years ago
|
Priority: -- → P2
Reporter | ||
Updated•22 years ago
|
Priority: P2 → P1
Reporter | ||
Comment 1•22 years ago
|
||
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.
Reporter | ||
Comment 2•22 years ago
|
||
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.
Reporter | ||
Comment 3•22 years ago
|
||
after analyze more carafully, I think we can have a more simple solution.
Reporter | ||
Comment 4•22 years ago
|
||
Reporter | ||
Comment 5•22 years ago
|
||
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.
Reporter | ||
Comment 6•22 years ago
|
||
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•22 years ago
|
||
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•22 years ago
|
||
This is major, not critical (no crash, hang or dataloss).
Severity: critical → major
Comment 9•22 years ago
|
||
Comment on attachment 110655 [details] [diff] [review]
set the title of main window to the opened message window
r=dmose
Attachment #110655 -
Flags: review+
Comment 10•22 years ago
|
||
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?
Reporter | ||
Comment 11•22 years ago
|
||
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
Reporter | ||
Comment 12•22 years ago
|
||
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?
Reporter | ||
Updated•22 years ago
|
Severity: critical → major
Comment 13•22 years ago
|
||
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•22 years ago
|
||
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)
Reporter | ||
Comment 15•22 years ago
|
||
up to now, reply issues still exist.
I want to do with it in another bug.
Reporter | ||
Comment 16•22 years ago
|
||
I filed a new bug http://bugzilla.mozilla.org/show_bug.cgi?id=187852 for the
reply issues.
Comment 17•22 years ago
|
||
I still think we should clone the dbview when we open a message window to
display the email.
Reporter | ||
Comment 18•22 years ago
|
||
Reporter | ||
Comment 19•22 years ago
|
||
Attachment #110655 -
Attachment is obsolete: true
Attachment #110745 -
Attachment is obsolete: true
Reporter | ||
Comment 20•22 years ago
|
||
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.
Reporter | ||
Updated•22 years ago
|
Attachment #110746 -
Flags: superreview?(sspitzer)
Attachment #110746 -
Flags: review?(dmose)
Comment 21•22 years ago
|
||
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.
Reporter | ||
Comment 22•22 years ago
|
||
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•22 years ago
|
||
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.
Reporter | ||
Comment 24•22 years ago
|
||
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•22 years ago
|
||
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.
Reporter | ||
Comment 26•22 years ago
|
||
Attachment #110746 -
Attachment is obsolete: true
Reporter | ||
Updated•22 years ago
|
Attachment #110856 -
Flags: review?(dmose)
Reporter | ||
Updated•22 years ago
|
Severity: major → critical
Reporter | ||
Updated•22 years ago
|
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)
Assignee | ||
Comment 27•22 years ago
|
||
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)
Assignee | ||
Comment 28•22 years ago
|
||
Attachment #110856 -
Attachment is obsolete: true
Assignee | ||
Comment 29•22 years ago
|
||
oops, I meant leon, not kyle.
thanks for the excellent bug report, leon.
can you review the patch?
Status: NEW → ASSIGNED
Assignee | ||
Comment 30•22 years ago
|
||
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.
Assignee | ||
Updated•22 years ago
|
Attachment #111075 -
Attachment description: patch based on kyle's work. → patch based on leon's work.
Assignee | ||
Comment 31•22 years ago
|
||
fixed. the patch has r/sr=bienvenu
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Reporter | ||
Comment 32•22 years ago
|
||
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.
Assignee | ||
Comment 33•22 years ago
|
||
*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.
Reporter | ||
Comment 34•22 years ago
|
||
Reporter | ||
Comment 35•22 years ago
|
||
seth,
I will file another bug to this optimization.
Reporter | ||
Comment 36•22 years ago
|
||
btw,
currenly, we can reuse the opened messgae window, so we also use the nonchnaged
Dbview.
Reporter | ||
Comment 37•22 years ago
|
||
The about optimization is 188459
Reporter | ||
Updated•22 years ago
|
Attachment #111126 -
Attachment is obsolete: true
Assignee | ||
Comment 38•22 years ago
|
||
thanks for logging the spin off bug. we'll take the discussion of the
optimization to that bug.
Comment 39•22 years ago
|
||
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
Updated•20 years ago
|
Product: Browser → Seamonkey
You need to log in
before you can comment on or make changes to this bug.
Description
•