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)

x86
All

Tracking

(Not tracked)

VERIFIED FIXED
mozilla0.9.5

People

(Reporter: darin.moz, Assigned: sspitzer)

References

Details

(Whiteboard: PDT+)

Attachments

(2 files)

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.
*** Bug 60515 has been marked as a duplicate of this bug. ***
OS: All.
OS: Linux → All
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. ***
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?
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
Havent seen this in a while either. Verified.
Status: RESOLVED → VERIFIED
Reproduced with 2001041808/Linux. Reopening.
Status: VERIFIED → REOPENED
Resolution: WORKSFORME → ---
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.
*** 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 ago23 years ago
Resolution: --- → WORKSFORME
WORKSFORME
Thanks Darin.
Status: RESOLVED → VERIFIED
*** Bug 79894 has been marked as a duplicate of this bug. ***
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 ago23 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
Status: VERIFIED → REOPENED
Resolution: WORKSFORME → ---
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
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.
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.
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.
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
we're leaking nsNNTPProtocol objects in this scenario, which is not good.

I've opened a new bug to cover that. see bug #100245
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.
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.
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.

I'll work this into 0.9.5
Target Milestone: --- → mozilla0.9.5
Summary: news displays message source when it shouldn't!! → news displays message source when it shouldn't
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.
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
*** Bug 95320 has been marked as a duplicate of this bug. ***
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+
Let's get it into the trunk today, test tomorrow then I'll go to PDT with it. 
Keywords: nsbranchnsbranch+
Blocks: 99508
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.
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.
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+
fixed on trunk and branch.
Status: ASSIGNED → RESOLVED
Closed: 23 years ago23 years ago
Resolution: --- → FIXED
Whiteboard: in the trunk, waiting to decide if it should be on the branch or not, PDT+ → PDT+
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
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: