Closed Bug 308025 Opened 19 years ago Closed 19 years ago

Crash if JavaScript Window.close() is executed within field onchange [@ nsEventStateManager::ShiftFocusInternal]

Categories

(Core :: DOM: UI Events & Focus Handling, defect)

Product:
Core
Component:
DOM: UI Events & Focus Handling
Platform:
x86
All
Type:
defect
Priority:
Not set
Severity:
critical

Tracking

()

Status:
VERIFIED FIXED

People

(Reporter: david, Assigned: MatsPalmgren_bugz)

Details

(Keywords: crash, testcase)

Crash Data

Attachments

(5 files, 4 obsolete files)

testcase
302 bytes, text/html
Details
another testcase
349 bytes, text/html
Details
stack + data
5.72 KB, text/plain
Details
Patch rev. 3
2.22 KB, patch
Details | Diff | Splinter Review
Patch rev. 3 (diff -w)
2.06 KB, patch
bzbarsky
: review+
bzbarsky
: superreview+
Details | Diff | Splinter Review 
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.6) Gecko/20050317 Firefox/1.0.2
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.6) Gecko/20050317 Firefox/1.0.2

A JavaScript like the following causes all instances of the browser to crash:
<input type="text" name="bogus" onchange="window.close();"/>
In my case I actually executed a seperate JavaScript function which had a
window.close() within it.

Reproducible: Always

Steps to Reproduce:
1.Enter something in the field and press tab to move to another field.

Actual Results:  
All instances of the browser crashed.

Expected Results:  
Gracefully close just the browser window the field was in.
Attached file testcase — 
Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8b4) Gecko/20050910
Firefox/1.4 ID:2005091004

WFM
Component: General → Layout
Product: Firefox → Core
QA Contact: general → layout
Version: unspecified → 1.7 Branch
testcase wfm Firefox 1.0.6, Seamonkey trunk.
Mozilla/5.0 (Windows; U; Win98; de-DE; rv:1.7.10) Gecko/20050715 Firefox/1.0.6
Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.9a1) Gecko/20050910 SeaMonkey/1.1a

I'm closing this bug as 'invalid', as you are using a more than half year old
Firefox 1.0.2. Feel free to reopen, if you can demonstrate a testcase crashing
on a current branch 1.0.6, or current beta, or current trunk nightlie.

http://www.mozilla.org/quality/bug-writing-guidelines.html
Status: UNCONFIRMED → RESOLVED
Closed: 19 years ago
Resolution: --- → INVALID
Ok, I downloaded and tested the same scenario under Firefox 1.5 beta 1. Still
blows the browser shutting down all instances. Below are two short HTML pages
that demonstrate the error.
testa.html:
<html>
  <script language="JavaScript">
    function doOpen() {
     
newwin=window.open('testb.html','dd','top=150,left=150,width=325,height=300',false);
      if (!newwin.opener) newwin.opener=self;
      newwin.focus();
    }
  </script>
  <body>
    <form action="test">
      <input type="button" value="Open Subwindow" onclick="doOpen();"/>
    </form>
  </body>
</html>

and testb.html:
<html>
  <body>
    <form action="test">
      <input type="text" onchange="window.close();"/>
    </form>
  </body>
</html>
Status: RESOLVED → UNCONFIRMED
Resolution: INVALID → ---
Summary: Crash is JavaScript Window.close() is executed within field onchange → Crash if JavaScript Window.close() is executed within field onchange
(In reply to comment #3)
> Ok, I downloaded and tested the same scenario under Firefox 1.5 beta 1. Still
> blows the browser shutting down all instances. Below are two short HTML pages
> that demonstrate the error.
> testa.html:
> <html>
>   <script language="JavaScript">
>     function doOpen() {
>      
>
newwin=window.open('testb.html','dd','top=150,left=150,width=325,height=300',false);
>       if (!newwin.opener) newwin.opener=self;
>       newwin.focus();
>     }
>   </script>
>   <body>
>     <form action="test">
>       <input type="button" value="Open Subwindow" onclick="doOpen();"/>
>     </form>
>   </body>
> </html>
> 
> and testb.html:
> <html>
>   <body>
>     <form action="test">
>       <input type="text" onchange="window.close();"/>
>     </form>
>   </body>
> </html>
> 

Your testcase tasta.html->testb.html still WFM

try in -safemode because it's most probably an extension that messes things up
Summary: Crash if JavaScript Window.close() is executed within field onchange → Crash is JavaScript Window.close() is executed within field onchange
Summary: Crash is JavaScript Window.close() is executed within field onchange → Crash if JavaScript Window.close() is executed within field onchange
Attached file another testcase — 
Crashes for me, too and I don't have any extensions except DOMI and Reporter
installed. Talkback id TB9250679Z.
Summary: Crash if JavaScript Window.close() is executed within field onchange → Crash if JavaScript Window.close() is executed within field onchange [@ nsEventStateManager::ShiftFocusInternal]
No crash when trying attachment 195670 [details] with Seamonkey 1.1a rv: 1.9a1 build
2005091105 under XP Pro SP2 here.
WFM
Assignee: nobody → mats.palmgren
Severity: normal → critical
Status: UNCONFIRMED → NEW
Component: Layout → Event Handling
Ever confirmed: true
Keywords: crash, testcase
OS: Windows XP → All
Version: 1.7 Branch → Trunk
Attached file stack + data — 

    
    

    
  

      
      
      
      

        Attached patch
          
          
        Patch rev. 1 (obsolete)
        — 
          — Splinter Review
      

    


  

    
    

    
  

      
      
      
      

        Attached patch
          
          
        Patch rev. 1 (diff -w) (obsolete)
        — 
          — Splinter Review
      

    


  

        Attachment #195700 -
        Flags: superreview?(bzbarsky)

        Attachment #195700 -
        Flags: review?(bzbarsky)

    
    

    
  
So why can't you pop out if GetCurrentDoc() is false?

    
    

    
  

      
      
      
      

        Attached patch
          
          
        Patch rev. 2 (obsolete)
        — 
          — Splinter Review
      

    


  
(In reply to comment #10)
> So why can't you pop out if GetCurrentDoc() is false?

ShiftFocus() with a start element that has been removed from the document
isn't useful, I think it will just degenerate into focusing the first element
in the document which probably isn't the right element in most situations.
In the past, we have favored not focusing anything at all rather than
possibly focusing the wrong element.

Here is an alternative patch that fixes the crash but still tries ShiftFocus
from possibly removed content.
I prefer the first patch though, or a combination of the two.

    
    

    
  

      
      
      
      

        Attached patch
          
          
        Patch rev. 2 (diff -w) (obsolete)
        — 
          — Splinter Review
      

    


  

        Attachment #195760 -
        Flags: superreview?(bzbarsky)

        Attachment #195760 -
        Flags: review?(bzbarsky)

    
    

    
  
The point is that just because GetCurrentDoc() is true doesn't mean the node
wasn't removed from the document then reinserted in a different location, or
even in a different document.  So whether you check GetCurrentDoc() or not it
would seem that you have the same problems...

    
    

    
  
Ok, so this should cover it then:
if (parentContainer && docContent && docContent->GetCurrentDoc() == parent_doc)

That would shift focus based on the new location in case it moved within the
document but I think we can live with that.

    
    

    
  
Yeah, if we're willing to move focus based on the new location that would work.

I still want to figure out a way to not fire events completely sync in cases
like this....

    
  

        Attachment #195700 -
        Attachment is obsolete: true

        Attachment #195700 -
        Flags: superreview?(bzbarsky)

        Attachment #195700 -
        Flags: review?(bzbarsky)

    
  

        Attachment #195760 -
        Attachment is obsolete: true

        Attachment #195760 -
        Flags: superreview?(bzbarsky)

        Attachment #195760 -
        Flags: review?(bzbarsky)

    
    

    
  

      
      
      
      

        Attached patch
          
          
        Patch rev. 3
        — 
          — Splinter Review
      

    


  

    
    

    
  


  

        Attachment #196004 -
        Flags: superreview?(bzbarsky)

        Attachment #196004 -
        Flags: review?(bzbarsky)

    
  

        Attachment #195759 -
        Attachment is obsolete: true

    
  

        Attachment #195699 -
        Attachment is obsolete: true

        Attachment #196004 -
        Flags: superreview?(bzbarsky)

        Attachment #196004 -
        Flags: superreview+

        Attachment #196004 -
        Flags: review?(bzbarsky)

        Attachment #196004 -
        Flags: review+

    
    

    
  
Checked in to trunk at 2005-09-13 22:03 PDT

-> FIXED
Status: NEW → RESOLVED
Closed: 19 years ago → 19 years ago
Resolution: --- → FIXED

    
    

    
  
Verified FIXED using the testcase at
https://bugzilla.mozilla.org/attachment.cgi?id=195670 with Mozilla/5.0 (Windows;
U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20050915 Firefox/1.6a1
Status: RESOLVED → VERIFIED
Crash Signature: [@  nsEventStateManager::ShiftFocusInternal]
Component: Event Handling → User events and focus handling

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






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: