Closed Bug 106558 Opened 24 years ago Closed 24 years ago

Mozilla briefly displays "<html><body></body><html>" during redirects

Categories

(Core :: DOM: Navigation, defect)

Product:
Core
Component:
DOM: Navigation
Platform:
x86
All
Type:
defect
Priority:
Not set
Severity:
critical

Tracking

()

Status:
RESOLVED FIXED

People

(Reporter: marc.loiselle, Assigned: rpotts)

References

(
URL
)

Details

(Keywords: regression, topembed)

Attachments

(1 file, 2 obsolete files)

an even better patch (that doesn't break multipart/x-mixed-replace)...
3.03 KB, patch
darin.moz
: review+
Details | Diff | Splinter Review
Redirects using Location http header result in Mozilla briefly displaying "<html><body></body><html>" Buildid 2001102403 on Windows I saw it on buildid 2001102203 on linux as well Related to bug 102737 Recent regression
been seeing that on linux too for the past days. Fine sample page is the CNN "Quickvote" window.
Status: UNCONFIRMED → NEW
Ever confirmed: true
looks like docshell or the uriloader code may not be handling NS_BINDING_REDIRECTED properly. cc'ing rpotts for help.
It's not just the <html><body></body></html> bits. Point a browser to: http://www.slashdot.org You get to see a lovely message about permanent redirection to http://slashdot.org which is well and good, but I haven't seen that message except for in the last few days, and I have a pretty good feeling that redirection's been around for a lot longer. I've seen this on both my Linux and Windows builds as well.
-> docshell
Assignee: darin → adamlock
Component: Networking: HTTP → Embedding: Docshell
QA Contact: tever → adamlock
-> rpotts
Assignee: adamlock → rpotts
Seeing this as well in a lot of different sites.
yea, this is all over the place. click on a link at http://my.netscape.com and you'll see it there too. upping severity, and nominating for mozilla0.9.6 I'm seeing this on today's mozilla win2k build.
Severity: normal → critical
Keywords: mozilla0.9.6, regression
I think bug 98203, bug 102737 and bug 106558 are all about the same thing. Do we want to concentrate on just one bug and mark the others DUPEs? I have been seeing this for a while on some pages if the connection times out or something, but recently (late october builds) it has appeared a lot more, most noticable when trying to go to http://www.hotmail.com/, since it redirects you somewhere else every time.
It appears that this problem was introduced on 10/18/01 in rev 1.59 of nsHTtpChannel.cpp. nsHTttpCHannel.cpp line 1236: mListener->OnStartRequest(...) was added. This exposed a problem upstream in the URILoader - which up until this point had only dealt with NS_BINDING_ABORTED / NS_ERROR_NO_CONTENT messages from OnStartRequest. Since NS_BINDING_REDIRECTED was *not* one of these two error messages, the notification was passed through. This caused the UnknownDecoder to be hooked up as the listener - because the content type of the redirected channel was *not* set. Because the stream was empty, the UnknownDecoder called the content type 'text/plain' and passed it to the parser. The parser received an OnStopRequest immediately following the OnStartRequest (with no calls to OnDataAvailable) os it dropped into some code which added '<html><body></body></html>' to the stream... Unfortunately, the content-type of the "empty" stream is text/plain *so* the 'empty html document is emited as plain text :-( what a mess !!!
I forgot to mention that the fix for this bug also addresses the issues of bug #72679
I would *really* like to remove ProcessCanceledCase(...) altogether. but i'm afraid that bug #40116 might reappear :-( -- rick
will this patch cause trouble for "host-not-found" errors that need to be propagated downstream?
no, because the OnStopRequest will *still* be propagated through... it is only the OnStartRequest that will be supressed in error conditions...
I've just attached a 'better' patch which removes ProcessCanceledCase(...) too... The main difference in behavior between this patch and the last one, is that if an error occurs on a channel, any pending data will be flushed through OnDataAvailable(...). Before, ProcessCanceledCase(...) would short-circuit the call. I believe that this is safe... To minimize possible circular references, i'm clearing out 'm_contextListener' and 'm_originalContext' in OnStopRequest (which is *always* called). still looking for r= and sr= :-)
*** Bug 72679 has been marked as a duplicate of this bug. ***
Blocks: 19073
Comment on attachment 56030 [details] [diff] [review] better patch that removes ProcessCanceledCase(... ) too... r=darin (this looks correct to me)
Attachment #56030 - Flags: review+
Comment on attachment 56030 [details] [diff] [review] better patch that removes ProcessCanceledCase(... ) too... sr=mscott although I'm nervous...our track record with changing this piece of good isn't very good =). Also, are you worried that by taking out the ProcessCanceledCase we are going to regress the original crash I was fixing with that? I forget the bug # but we can look it up in lxr if we have to.
Attachment #56030 - Flags: superreview+
hey scott, the original bug that ProcessCanceledCase(...) was added to fix was bug #40116. Since we are now short-circuiting OnStartRequest(...) for *any* failure - not just NS_BINDING_ABORTED - i believe that we should be safe. after all, we are just being *more* restrictive, not less :-) -- rick
I've checked the patch in.
Status: NEW → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
Is this bug related to "Object Moved" message, displayed briefly before redirecting ? This message should be removed, too, because many web developers rely heavily on redirecting mechanism, and this message ruins "continious browsing" fealing.
Reopening. This caused bug 108869.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Attachment #55644 - Attachment is obsolete: true
Attachment #56030 - Attachment is obsolete: true
Comment on attachment 56942 [details] [diff] [review] an even better patch (that doesn't break multipart/x-mixed-replace)... of course!! (r/sr=darin)
Attachment #56942 - Flags: review+
*** Bug 109329 has been marked as a duplicate of this bug. ***
I've just checked the 'better patch' in. i believe that everything should go smoothly with this one :-)
Status: REOPENED → RESOLVED
Closed: 24 years ago → 24 years ago
Resolution: --- → FIXED
Is this good enough for 0.9.6?
Blocks: 104864
nominating for EDT.
Keywords: edt0.9.4
this should be fine for 0.9.6... however, the fix for bug 108267 also went in before branching for 0.9.6, and it should have also eliminated the temporary <html><body></body><html> page shown during redirects (since bug 108267 covered the cause of the regression). so, unless this patch helps with other bugs, i don't see it being quite so critical anymore.
Well, using the test case in the bug I don't see anything during the redirect. Was the bug really obvious using the URL above? If it's not a big deal then I'll leave it be.
No longer blocks: 104864
Keywords: mozilla0.9.6+
we're not experiencing this empty/tag doc stuff in 0.9.4, minusing.
Keywords: edt0.9.4 → edt0.9.4-
EDT - marking this with a plus.
Keywords: edt0.9.4- → edt0.9.4+
Tested on the following: 11_14_22_0.9.4ec build on Win98 11_28_22_0.9.4ec build on Win2000 12_04_10_trunk build on Win2000 12_04_08_trunk build on Linux Tested using these sites: http://www.myforums.com http://my.netscape.com http://www.slashdot.org Also tested using a bogus ftp address where I previously saw this problem. Results: No evidence of '<html><body></body><html>' display in any testing.
*** Bug 113573 has been marked as a duplicate of this bug. ***
that's what I thought as well. minusing.
Keywords: edt0.9.4+ → edt0.9.4-
Keywords: topembed
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: