Closed Bug 194329 Opened 22 years ago Closed 22 years ago

crash in [@ SinkContext::AddComment]

Categories

(Core :: DOM: HTML Parser, defect, P1)

Product:
Core
Component:
DOM: HTML Parser
Platform:
x86
All
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla1.4alpha

People

(Reporter: ajschult784, Assigned: harishd)

References

(
URL
)

Details

(4 keywords, Whiteboard: [fixed on the trunk and branch] fixed1.3)

Crash Data

Attachments

(3 files, 1 obsolete file)

testcase
274 bytes, text/html
Details
stacktrace
3.31 KB, text/plain
Details
patc v1.1
2.18 KB, patch
hjtoi-bugzilla
: review+
jst
: superreview+
asa
: approval1.3+
Details | Diff | Splinter Review 
from bug 188474
------- Additional Comment #21 From Jay Patel  2003-02-18 13:18 -------

Reopening for now to see what everyone else thinks...but I just crashed with a
similar stacktrace going to
http://cgi.ebay.com/ws/eBayISAPI.dll?ViewItem&item=3008284272&category=15046 .

--------------------------------------

I'll attach a testcase for the new URL.  It appears to be different than the one
from bug 188474.  Loading the testcase in a debug build produces these
assertions before crashing:

###!!! ASSERTION: leaf w/o container: 'mStackPos > 0', file
nsHTMLContentSink.cpp, line 2220
###!!! ASSERTION: container w/o parent: 'mStackPos > 0', file
nsHTMLContentSink.cpp, line 1680
Attached file testcase — 
crashes linux trunk build 20030220
Attached file stacktrace — 

    
    

    
  
backing out bug 187790 fixes the crash and assertions
Keywords: regression, 
          
            testcase

    
    

    
  
crash on testcase:
Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.3b) Gecko/20030221
Talkback couldn´t connect to server, also doesn´t show details.

    
  
Status: NEW → ASSIGNED
Priority: -- → P1
Target Milestone: --- → mozilla1.4alpha

    
    

    
  
Andrew: Thanks for the testcase. I was able to crash as well. Will take a look
today.

    
    

    
  
Making topcrash+ since we have a reproducible testcase.
Keywords: topcrash → topcrash+
Summary: crash in SinkContext::AddComment → crash in [@ SinkContext::AddComment]

    
    

    
  

      
      
      
      

        Attached patch
          
          
        patch v1.0 (obsolete)
        — 
          — Splinter Review
      

    


  
Problem: The head context, that was opened up by <script>, did not get closed
before handling the document.write content ( which is just </div> in the
testcase provided ). This caused the head context's stack position to be
altered.

Fix: Check if the current context is head context, by calling
CloseHeadContext(), before closing a container.

    
  
Whiteboard: [fix in hand]

    
    

    
  

      
      
      
      

        Attached patch
          
          
        patc v1.1
        — 
          — Splinter Review
      

    


  
This patch prevents <script> from opening up a head context - happened only if
a <head> was found after <body> ( not sure why we had to do that. I tracked it
down to revision 3.263 of CNavDTD but the checkin comment was not of any help )
- if <script> were to be in the body context; for compatibility we allow
<script> to appear anywhere in the document. I'll have to run parser regression
tests to confirm this patch.

        Attachment #115443 -
        Attachment is obsolete: true

    
    

    
  
Any chance of getting this into Mozilla 1.3?  It's topcrash #10 with Mozilla 1.3
Beta.  Should we nominate it?

    
  
Flags: blocking1.3?

    
    

    
  
Comment on attachment 115557 [details] [diff] [review]
patc v1.1

Passed parser regression tests.

    
  

        Attachment #115557 -
        Flags: superreview?(jst)

        Attachment #115557 -
        Flags: review?(heikki)

    
    

    
  
Comment on attachment 115557 [details] [diff] [review]
patc v1.1

sr=jst.

I think it would be worth adding some assertions and some more band-aid code to
the sink though to deal with possibly similar errors that could creep up.
Please file a new bug on making sure the sink plays nicer and doesn't drop
errors like it does today (I have a patch that fixes part of this already, let
me know when you've filed a bug and I'll attach it).

        Attachment #115557 -
        Flags: superreview?(jst) → superreview+

    
    

    
  
With a fix in hand, setting to blocking 1.3 with the hope that we can get this
into the branch for the release.
Flags: blocking1.3? → blocking1.3+

        Attachment #115557 -
        Flags: review?(heikki) → review+

    
    

    
  
Comment on attachment 115557 [details] [diff] [review]
patc v1.1

a=asa (on behalf of drivers) for checkin to the 1.3 branch.

        Attachment #115557 -
        Flags: approval1.3+

    
  
Whiteboard: [fix in hand] → [fixed on the trunk]

    
    

    
  
Fixed on the trunk and branch.
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Whiteboard: [fixed on the trunk] → [fixed on the trunk and branch]

    
  
Whiteboard: [fixed on the trunk and branch] → [fixed on the trunk and branch] fixed1.3

    
    

    
  
*** Bug 196762 has been marked as a duplicate of this bug. ***

    
    

    
  
*** Bug 196762 has been marked as a duplicate of this bug. ***

    
    

    
  
Crashtest added as part of http://hg.mozilla.org/mozilla-central/rev/afc662d52ab1
Flags: in-testsuite+
Crash Signature: [@ SinkContext::AddComment]

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: