Closed Bug 942979 Opened 11 years ago Closed 11 years ago

Crash in nsContentUtils::GetCommonAncestor

Categories

(Core :: DOM: Core & HTML, defect)

Product:
Core
Component:
DOM: Core & HTML
Version:
25 Branch
Platform:
x86_64
Windows 7
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla28

People

(Reporter: vulnerable.zappa, Assigned: mz_mhs-ctb)

Details

Attachments

(2 files, 3 obsolete files)

repro.html
1.00 KB, text/html
Details
Patch with test (v2). r=bzbarsky
2.83 KB, patch
Details | Diff | Splinter Review
Attached file repro.html — 
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/30.0.1599.101 Safari/537.36 OPR/17.0.1241.53

Steps to reproduce:

run repro.html


Actual results:

Firefox crash


Expected results:

Nothing
Confirmed locally.
Assignee: nobody → mz_mhs-ctb
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Summary: nsDocument::mSubtreeModifiedTargets crash → Crash in nsContentUtils::GetCommonAncestor
Attached patch Patch (obsolete) — — Splinter Review 

        Attachment #8339661 -
        Flags: review?(bzbarsky)

    
    

    
  
Comment on attachment 8339661 [details] [diff] [review]
Patch

Why is mSubtreeModifiedTargets[i] null, exactly?

    
    

    
  
That is, the real bug is whatever allows a null to end up in there; there should be no null values in that array.

    
  

        Attachment #8339661 -
        Flags: review?(bzbarsky)

    
    

    
  

      
      
      
      

        Attached patch
          
          
        Patch v2 (obsolete)
        — 
          — Splinter Review
      

    


  

        Attachment #8339661 -
        Attachment is obsolete: true

        Attachment #8340541 -
        Flags: review?(bzbarsky)

    
    

    
  
Comment on attachment 8340541 [details] [diff] [review]
Patch v2

Yes, thank you. This makes a lot more sense.  Maybe add a comment about how parentNode can be null if an earlier mutation event removed the node?

r=me

        Attachment #8340541 -
        Flags: review?(bzbarsky) → review+
Component: Untriaged → DOM
Product: Firefox → Core

    
    

    
  
And add the test? :)

    
    

    
  

      
      
      
      

        Attached patch
          
          
        Patch with test. (obsolete)
        — 
          — Splinter Review
      

    


  

        Attachment #8340541 -
        Attachment is obsolete: true

    
    

    
  
Comment on attachment 8340678 [details] [diff] [review]
Patch with test.

Carrying r+ from bzbarsky.

        Attachment #8340678 -
        Flags: review+

    
  
Keywords: checkin-needed

    
    

    
  


  

        Attachment #8340678 -
        Attachment is obsolete: true

    
    

    
  
Sorry about that, forgot the comment.

    
    

    
  
https://hg.mozilla.org/mozilla-central/rev/bfa858a90cb0
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
Component: DOM → DOM: Core & HTML

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






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: