Closed Bug 897399 Opened 11 years ago Closed 11 years ago

Crash with showModalDialog in beforescriptexecute handler

Categories

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

Product:
Core
Component:
DOM: Core & HTML
Type:
defect
Priority:
Not set
Severity:
critical

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla26
Tracking Flags:
Tracking Status
firefox25 + verified
firefox26 --- verified

People

(Reporter: jruderman, Assigned: Gavin)

References

Details

(4 keywords)

Crash Data

Attachments

(3 files, 1 obsolete file)

testcase
596 bytes, text/html
Details
stacks
35.00 KB, text/plain
Details
patch
963 bytes, patch
sicking
: review+
akeybl
: approval-mozilla-aurora+
Details | Diff | Splinter Review
Attached file testcase — 
###!!! ASSERTION: shouldn't be called with a null inner window: 'pwin', file ../../../../content/base/src/nsScriptLoader.cpp, line 815

###!!! ASSERTION: windows must be global objects: 'globalObject', file ../../../../content/base/src/nsScriptLoader.cpp, line 818

Probably a regression from bug 883592.
Attached file stacks — 

    
  
Crash Signature: [@ nsScriptLoader::EvaluateScript(nsScriptLoadRequest*, nsString const&)]

    
    

    
  

      
      
      
      

        Attached patch
          
          
        patch (obsolete)
        — 
          — Splinter Review
      

    


  
I imagine something like this is needed, but this might not be quite right (not sure what the behavior should be in this case).

    
    

    
  

      
      
      
      

        Attached patch
          
          
        patch
        — 
          — Splinter Review
      

    


  
(attached the wrong patch)

        Attachment #780697 -
        Attachment is obsolete: true

        Attachment #780698 -
        Flags: review?(jonas)

    
    

    
  
The other alternative would be to restore the null check in EvaluateScript (i.e. undo the second hunk of revision 19d5caf61217), but that leads to more unnecessary work being done.
Assignee: nobody → gavin.sharp
OS: Mac OS X → All
Hardware: x86_64 → All

    
    

    
  
https://hg.mozilla.org/integration/fx-team/rev/bfdaf248533b
Flags: in-testsuite?
Target Milestone: --- → mozilla26

    
    

    
  
Comment on attachment 780698 [details] [diff] [review]
patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 883592
User impact if declined: non-security crashes in some edge cases
Testing completed (on m-c, etc.): 
Risk to taking this patch (and alternatives if risky): just a null check, low risk
String or IDL/UUID changes made by this patch: none

        Attachment #780698 -
        Flags: approval-mozilla-aurora?

    
    

    
  
https://hg.mozilla.org/mozilla-central/rev/bfdaf248533b
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED

    
    

    
  
Comment on attachment 780698 [details] [diff] [review]
patch

Sorry for the delayed approval, extremely low risk fix for a new regression.

        Attachment #780698 -
        Flags: approval-mozilla-aurora? → approval-mozilla-aurora+

    
    

    
  
Reproduced in 2013-07-24-mozilla-central-debug Mac OS X 10.8.4.
Verified fixed FF 25 2013-10-01-mozilla-beta-debug.

          status-firefox25:
          fixed → verified

    
    

    
  
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:26.0) Gecko/20100101 Firefox/26.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:26.0) Gecko/20100101 Firefox/26.0
Mozilla/5.0 (X11; Linux i686; rv:26.0) Gecko/20100101 Firefox/26.0

Verified as fixed on latest Aurora 26.0a2 (buildID: 20131011004001).
Status: RESOLVED → VERIFIED

          status-firefox26:
          fixed → verified
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: