Closed Bug 308025 Opened 20 years ago Closed 20 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: 20 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: 20 years ago → 20 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: