Closed Bug 106558 Opened 23 years ago Closed 23 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)...
(deleted), 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: 23 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: 23 years ago → 23 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: