Closed
Bug 59449
Opened 24 years ago
Closed 23 years ago
news displays message source when it shouldn't
Categories
(SeaMonkey :: MailNews: Message Display, defect, P3)
Tracking
(Not tracked)
VERIFIED
FIXED
mozilla0.9.5
People
(Reporter: darin.moz, Assigned: sspitzer)
References
Details
(Whiteboard: PDT+)
Attachments
(2 files)
97.35 KB,
image/png
|
Details | |
820 bytes,
patch
|
mscott
:
review+
mscott
:
superreview+
|
Details | Diff | Splinter Review |
From Bugzilla Helper: User-Agent: Mozilla/5.0 (X11; U; Linux 2.2.16-22 i686; en-US; m18) Gecko/20001107 Netscape6/6.0 BuildID: 2000110709 Click on a news message... before it loads, click on another message. This puts news in a BAD state. Now, if you click on a message, the message does not display correctly; the message source is displayed instead of the rendered message. Reproducible: Always Steps to Reproduce: 1.Click on an unread news message 2.Before it loads, click on a different unread news message 3.Let it finish loading. 4.Now click on a different unread news message. 5.Notice the junk (message source) displayed on the screen. Actual Results: Displays message source on the screen! Expected Results: Displays message without headers and such.
Reporter | ||
Comment 1•24 years ago
|
||
Comment 4•24 years ago
|
||
reassigning to sspitzer, ccin'g bienvenu. I think this is a dup.
Assignee: putterman → sspitzer
Taking...
QA Contact: esther → stephend
*** Bug 64740 has been marked as a duplicate of this bug. ***
Comment 7•23 years ago
|
||
unable to test this because of bug 71024 but I haven't seen this problem in a long time. Darin, have you seen this recently?
Reporter | ||
Comment 8•23 years ago
|
||
no.. actually i haven't seen this in a while.
marking worksforme. we can re-open when (and if) we see this after the re-write lands.
Status: NEW → RESOLVED
Closed: 23 years ago
Resolution: --- → WORKSFORME
Comment 11•23 years ago
|
||
Reproduced with 2001041808/Linux. Reopening.
Status: VERIFIED → REOPENED
Resolution: WORKSFORME → ---
See also http://bugzilla.mozilla.org/show_bug.cgi?id=76558 (but they're not DUPs).
Actually, they might be very closely (or indeed a DUP), but until I hear from a developer that the underlying code calls are the same, I'll let this one stand open. Pretty sure this is cache.
Comment 14•23 years ago
|
||
*** Bug 78970 has been marked as a duplicate of this bug. ***
I have not seen this lately, with the exception of the known and fixed cache bug that mscott fixed. marking worksforme. I rapidly selected news messages before the previous one had finished loading. I think also our persistence of headers (or the way we cause it to reflow) has helped out here.
Status: REOPENED → RESOLVED
Closed: 23 years ago → 23 years ago
Resolution: --- → WORKSFORME
Reporter | ||
Comment 16•23 years ago
|
||
WORKSFORME
Thanks Darin.
Status: RESOLVED → VERIFIED
Comment 18•23 years ago
|
||
*** Bug 79894 has been marked as a duplicate of this bug. ***
Comment 19•23 years ago
|
||
Can anybody test this case? Bug 79894 was reported. 2001050910/Linux displays nothing in step 5 of the original report.
Status: VERIFIED → REOPENED
Resolution: WORKSFORME → ---
This was fixed on the trunk builds, try with a nightly open only if you see it with a current *trunk* build. See bug 76558 for details.
Status: REOPENED → RESOLVED
Closed: 23 years ago → 23 years ago
Resolution: --- → WORKSFORME
This is still working for me with builds: Mac OS 9.1 2001-05-11-09 RedHat 7.0 2001-05-11-08 Windows 2000 2001-05-11-04
Status: RESOLVED → VERIFIED
Updated•23 years ago
|
Status: VERIFIED → REOPENED
Resolution: WORKSFORME → ---
Comment 22•23 years ago
|
||
Although it is very rare, i still see that problem with latest nighties. Problem is maybe i have a slow connection (33kbps modem) and i am a fast mouse clicker.. Mozilla/5.0 (Windows; U; Win98; en-US; rv:0.9.1+) Gecko/20010616
Comment 23•23 years ago
|
||
It seems as if it is triggered by switching connection for loading. When using the normal first connection everything works fine, but if you interrupt an operation so that a new connection is used while the first one is finishing the cancelled operation, I see this problem. If I produce another connection switch (back to the original?) the problem is gone. This is not 100% reproducible but it could be that something isn't equal among the connections, or there are some kind of connection mixup somewhere.
Comment 24•23 years ago
|
||
I have a proposed fix for bug 95320 which has the side effect of provoking this bug more often. Unlike Daniel, I usually have to make two or more connection switches to trigger the bug. I don't understand the code well enough to explain exactly what's going on, but when the user switches away from a connection we end up, via nsNNTPProtocol::OnStopRequest, in nsNNTPProtocol::CloseSocket, which (IMHO) does not clean up as thoroughly as it should. Hence, bad things happen when this instance is reused. My empirical fix, based on what nsNNTPProtocol::CloseConnection does, is to add a call to m_nntpServer->RemoveConnection. I'm attaching a patch to show what I mean. I feel sure that someone with a better understanding can improve on this, but it's a step in the right direction.
Comment 25•23 years ago
|
||
Comment 26•23 years ago
|
||
I've used your patch during the weekend and it works fine. I haven't seen any post displayed as raw source since applying it. It also seems to fix the long hangs I've seen quite often, where the post isn't shown for half a minute or so. I always thought that it was network trouble but maybe it was just Mozilla abusing the network in some way. I would really like this go in soon. Maybe even for 0.9.4 if that is possible. sspitzer? bienvenue? Håkan? Can you review it? P.S. I also have the one line patch for bug 95320 in my tree. It might be necessary to have both patches to avoid all problems. D.S.
Assignee | ||
Comment 27•23 years ago
|
||
I was able to reproduce the problem and your patch has fixed it. the socket is getting closed because we are handling a nsOnStopRequestEvent, I'm not sure who is posting the event yet. nsNNTPProtocol::CloseSocket() line 5421 nsNNTPProtocol::OnStopRequest(nsNNTPProtocol * const 0x05aa19ec, nsIRequest * 0x05ad7750, nsISupports * 0x05aa43a0, unsigned int 2152398850) line 1188 nsOnStopRequestEvent::HandleEvent() line 162 nsARequestObserverEvent::HandlePLEvent(PLEvent * 0x05fb9fc4) line 65 PL_HandleEvent(PLEvent * 0x05fb9fc4) line 590 + 10 bytes PL_ProcessPendingEvents(PLEventQueue * 0x00572900) line 520 + 9 bytes _md_EventReceiverProc(HWND__ * 0x001e0358, unsigned int 49381, unsigned int 0, long 5712128) line 1071 + 9 bytes USER32! 77e13eb0() USER32! 77e1401a() USER32! 77e192da() nsAppShellService::Run(nsAppShellService * const 0x0176cb60) line 453 main1(int 4, char * * 0x00484090, nsISupports * 0x00000000) line 1268 + 32 bytes main(int 4, char * * 0x00484090) line 1590 + 37 bytes mainCRTStartup() line 338 + 17 bytes KERNEL32! 77e87903() if we close the socket, the connection should no longer be cached. keeping it cached will lead to several problems. related to why stop even, I get assertions soon after: NTDLL! 77f9f9df() nsDebug::Assertion(const char * 0x01d4f678, const char * 0x01d4f634, const char * 0x01d4f5fc, int 463) line 290 + 13 bytes nsDocLoaderImpl::OnStartRequest(nsDocLoaderImpl * const 0x045d0114, nsIRequest * 0x05fcb2d0, nsISupports * 0x00000000) line 463 + 54 bytes nsLoadGroup::AddRequest(nsLoadGroup * const 0x045d00a0, nsIRequest * 0x05fcb2d0, nsISupports * 0x00000000) line 454 + 40 bytes nsMsgProtocol::OnStartRequest(nsMsgProtocol * const 0x05fcb2cc, nsIRequest * 0x05fbed70, nsISupports * 0x05fcd9a0) line 262 nsOnStartRequestEvent::HandleEvent() line 110 + 53 bytes nsARequestObserverEvent::HandlePLEvent(PLEvent * 0x05fa37c4) line 65 PL_HandleEvent(PLEvent * 0x05fa37c4) line 590 + 10 bytes PL_ProcessPendingEvents(PLEventQueue * 0x00572900) line 520 + 9 bytes _md_EventReceiverProc(HWND__ * 0x001e0358, unsigned int 49381, unsigned int 0, long 5712128) line 1071 + 9 bytes USER32! 77e13eb0() USER32! 77e1401a() USER32! 77e192da() nsAppShellService::Run(nsAppShellService * const 0x0176cb60) line 453 main1(int 4, char * * 0x00484090, nsISupports * 0x00000000) line 1268 + 32 bytes main(int 4, char * * 0x00484090) line 1590 + 37 bytes mainCRTStartup() line 338 + 17 bytes KERNEL32! 77e87903()
Status: REOPENED → ASSIGNED
Assignee | ||
Comment 28•23 years ago
|
||
we're leaking nsNNTPProtocol objects in this scenario, which is not good. I've opened a new bug to cover that. see bug #100245
Assignee | ||
Comment 29•23 years ago
|
||
also related to this bug, we shouldn't mark the message as read if we "zoom" over it when holding space bar or n (next message). I've logged bug #100246 to track this, but that bug may be a duplicate.
Comment 30•23 years ago
|
||
The stop event may be caused by nsNntpIncomingServer::ConnectionTimeOut. I noticed that lastActiveTimeStamp here is often zero. Not sure what this is all about, or why we should close a connection to the news server after 170 seconds of inactivity.
Assignee | ||
Comment 31•23 years ago
|
||
I think your fix for #95320 isn't necessary. once you've removed the connection from the cache, it won't get re-used. > lastActiveTimeStamp here is often zero. that's probably because the connection was never actually used. > about, or why we should close a connection to the news server after 170 seconds > of inactivity. we do that because if you haven't used the connection in 170 seconds, we assume it's not longer active because the server dropped us. we have a similar timeout (albeit longer, I think 29 minutes) for IMAP.
Assignee | ||
Updated•23 years ago
|
Summary: news displays message source when it shouldn't!! → news displays message source when it shouldn't
Comment 33•23 years ago
|
||
ok, r=bienvenu, but we need to figure out why OnStopRequest isn't getting called in the normal case (which is a good thing at the moment, or the connection cache wouldn't work) and fix whatever leaks we have. However, this patch should help, and does no additional harm.
Assignee | ||
Comment 34•23 years ago
|
||
I agree with bienvenu on all points. sr=sspitzer. I'll get this into the trunk and see if we can get it on the branch. then later, I'll do the investigation that bienvenu suggests.
Keywords: nsbranch
Assignee | ||
Comment 35•23 years ago
|
||
*** Bug 95320 has been marked as a duplicate of this bug. ***
Comment 36•23 years ago
|
||
Comment on attachment 48605 [details] [diff] [review] A rather tentative patch this has been reviewed and super reviewed.
Attachment #48605 -
Flags: superreview+
Attachment #48605 -
Flags: review+
Comment 37•23 years ago
|
||
Let's get it into the trunk today, test tomorrow then I'll go to PDT with it.
Assignee | ||
Comment 38•23 years ago
|
||
in the trunk, waiting to decide if it should be on the branch or not.
Whiteboard: in the trunk, waiting to decide if it should be on the branch or not.
Comment 39•23 years ago
|
||
Seth, I agree that my fix for #95320 is no longer necessary. Taking the discussion about the 170s timeout to bug 100522.
It is my opinion after testing this on the trunk, that this should go to the branch. It hasn't caused any regressions, but even with the patch there is a minor issue (that is probably due to the timer code, and the way we mark messages, as Seth has mentioned elsewhere): If you press and hold the spacebar down, anywhere from 1-3 messages out of 30 or so will display our "Expired Article" page, but all the rest of the messages display fine. And, I can't imagine someone legitimately using "Advance to next unread message" by simply holding down the spacebar. It seems to be that we're just going too fast, and we partially mark it in the cache (sorry, that's my best QA explanation). Anyway, the above won't happen to the average user who just presses the spacebar a couple or one at a time. Builds: Mac OS 9.1 - 2001-09-19-08 RedHat 7.1 - 2001-09-19-08 Windows 2K - 2001-09-19-03
A little clarification, I was testing this in netscape.public.mozilla.mail-news, where I don't think messages are set to expire.
This patch seems to really help bug 97422.
Comment 43•23 years ago
|
||
check it in - PDT+
Whiteboard: in the trunk, waiting to decide if it should be on the branch or not. → in the trunk, waiting to decide if it should be on the branch or not, PDT+
Assignee | ||
Comment 44•23 years ago
|
||
fixed on trunk and branch.
Status: ASSIGNED → RESOLVED
Closed: 23 years ago → 23 years ago
Resolution: --- → FIXED
Whiteboard: in the trunk, waiting to decide if it should be on the branch or not, PDT+ → PDT+
Comment 45•23 years ago
|
||
Phil Anderton -- thanks!
Okay, I beat news to death (without doing the known spacebar bug) and couldnt' get to that bogus cache/screwed up state on: 2001-09-25 (branch builds) with: Mac OS 9.1, Windows 2000 and RedHat 7.1. Fixed, since I last verified it on the trunk.
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
•