Closed Bug 293388 Opened 19 years ago Closed 19 years ago

Reentering nsRange::DeleteContents for same-ish range crashes

Categories

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

Product:
Core
Component:
DOM: Core & HTML
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla1.8beta4

People

(Reporter: adrenalinup, Assigned: bzbarsky)

References

(
URL
)

Details

(Keywords: crash, testcase)

Attachments

(3 files, 2 obsolete files)

testcase
706 bytes, text/html
Details
Testcase that makes it clearer what's going on
719 bytes, text/html
Details
Proposed patch
3.02 KB, patch
sicking
: review+
peterv
: superreview+
benjamin
: approval1.8b4+
Details | Diff | Splinter Review 
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.7) Gecko/20050414 Firefox/1.0.3
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b2) Gecko/20050508

When innerHTML of a div is overwrited by innerHTML of a iframe, the loading icon
starts and can't be stoped, on multiple tryes browser can crash.
I have created a loop function what call the function multiple times, to crash
the browser..
The main problem is the loading icon.. I really need that script for my site(i
trying to create a iframe with dynamic width&height)..

Reproducible: Sometimes

Steps to Reproduce:
1. Execute the function what overwrite the DIV innerHTML (after that, the
loading icon starts and can't be stoped)
2. Execute multiple times.. You can do that with a loop.. (See my url)
3.

Actual Results:  
Firstly the loadin animation can't be stopped..
If you repeat multiple times, browser can crash.

Expected Results:  
The loading animation should stop after loading
Browser shouldn't crash

Here is ff_crash.html

<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN">
<html><head>
<title>Crasher</title>

</head><body>

<div id="dynamicdiv">
<iframe onLoad="insertIt();" name="dynamicframe" scrolling="no" frameborder="0"
src="blank.html" height="100%" width="100%"></iframe>
</div>


<script language="JavaScript">
function insertIt() {
	var _y = document.getElementById('dynamicdiv');
	var _x = window.frames[0].document.body.innerHTML;
	_y.innerHTML = _x;
}

function makeIframe() {
	var newIframe = document.createElement('iframe');
	newIframe.onload = function() { insertIt(); }
	newIframe.name ='dynamicframe';
	newIframe.scrolling='no'; 
	newIframe.frameborder='0';
	newIframe.src ='blank.html';
	newIframe.height='100%';
	newIframe.width='100%';
	document.getElementById('dynamicdiv').appendChild(newIframe);
}

function Loop() {
for(i = 1; i <= 30; i++) makeIframe();
}
</script>


<a onclick="Loop();">Press to Crash</a>
</body</body></html>


Another version.. Also cause to crash..
Insted of function makeIframe() {
	var newIframe = document.createElement('iframe');
	newIframe.onload = function() { insertIt(); }
	newIframe.name ='dynamicframe';
	newIframe.scrolling='no'; 
	newIframe.frameborder='0';
	newIframe.src ='blank.html';
	newIframe.height='100%';
	newIframe.width='100%';
	document.getElementById('dynamicdiv').appendChild(newIframe);
}

Also can be ussed..
function makeIframe() {
	var _y = document.getElementById('dynamicdiv');
	_y.innerHTML = '<iframe onLoad="insertIt();" name="dynamicframe" scrolling="no"
frameborder="0" src="blank.html" height="100%" width="100%"></iframe>';
}

Same too be same resultat..

Here is Mozilla crash info.. A bit long.. Oh.. Too long.. If you still need it,
send me a email..

Application Information

 Application Launch Time
   08.05.2005 18:19

 Build Identifier
   2005050805

 Deployment Identifier
   MozillaOrgMozillaTrunkWin322005050805

 Interface Version
            8 (0x00000008)

 Monitor Configuration Version
            1 (0x00000001)

 Platform Identifier
   Win32

 Product Identifier
   MozillaTrunk

 Talkback Configuration Version
            1 (0x00000001)

 Talkback Library Version
            7 (0x00000007)

 Vendor Identifier
   MozillaOrg

Runtime Information

 Active Application
         2084 (0x00000824)

 Command Line
   "C:\Program Files\mozilla.org\Mozilla\Mozilla.exe" -installer

 Contents of the Stack

[   0] 10 10 00 00 00 10 00 00 B0 53 12 00 00 10 00 00 [.........S......]
[  10] A8 5A E8 01 00 00 00 00 80 CA A3 00 00 00 00 00 [.Z..............]
[  20] F4 53 12 00 B3 BB 26 60 00 00 00 00 64 10 11 61 [.S....&`....d..a]
[  30] 00 EE 32 01 00 00 00 00 F0 C7 E8 01 01 00 00 00 [..2.............]
[  40] 74 AB E8 01 00 00 00 00 00 26 E2 01 8F 70 15 60 [t........&...p.`]
[  50] 00 00 00 00 10 54 12 00 6F 51 2A 60 70 AB E8 01 [.....T..oQ*`p...]
[  60] 00 00 00 00 00 EE 32 01 14 CB CF 01 00 26 E2 01 [......2......&..]
[  70] 68 54 12 00 6D 12 29 60 A8 5A E8 01 88 1C B5 01 [hT..m.)`.Z......]
[  80] 2C 12 C3 01 18 50 E5 01 00 00 00 00 33 C7 25 60 [,....P......3.%`]
[  90] 00 00 00 00 2C 12 C3 01 00 EE 32 01 10 12 C3 01 [....,.....2.....]
[  A0] 6C 54 12 00 88 1C B5 01 00 00 00 00 00 00 00 00 [lT..............]
[  B0] 01 00 00 00 00 00 00 00 18 50 E5 01 C8 ED 32 01 [.........P....2.]
[  C0] 50 50 E5 01 14 CB CF 01 88 54 12 00 7C 0F 29 60 [PP.......T..|.)`]
[  D0] 00 EE 32 01 01 00 00 00 2C 12 C3 01 18 50 E5 01 [..2.....,....P..]
[  E0] 2C 12 C3 01 00 00 00 00 A4 54 12 00 02 07 27 60 [,........T....'`]
[  F0] 18 50 E5 01 00 00 00 00 00 EE 32 01 B8 EC 32 01 [.P........2...2.]
[ 100] C8 AC E1 01 B4 55 12 00 D5 CA 25 60 18 50 E5 01 [.....U....%`.P..]
[ 110] 00 00 00 00 00 EE 32 01 10 12 C3 01 88 12 C3 01 [......2.........]
[ 120] B8 E7 BD 01 58 BC CE 01 00 00 00 00 58 BC CE 01 [....X.......X...]
[ 130] 34 98 11 60 C9 C3 C2 77 00 00 27 00 00 00 00 00 [4..`...w..'.....]
.............
[  60] 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 [................]
[  70] 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 [................]
[  80] 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 [................]
[  90] 00 00 00 00                                     [....]

Thanks guys ;)
I'm from Moldova (if that matter ;o)
If you have a workarround.. Please let me know.. I need a solution to make that
code work properly on all browsers(on IE works just fine).. Need an
autoresizeble iframe..
reporter: please run components\talkback and copy the incident id.

also, please load this bug and click 'create a new attachment' and attach your
crashing html.
Assignee: general → general
Component: JavaScript Engine → DOM: HTML
QA Contact: general → ian
I got a crash with a 2005-05-07 trunk build. Talkback ID: TB5686535Q

nsRange::DeleteContents 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/content/base/src/nsRange.cpp,
line 1591]
nsGenericHTMLElement::SetInnerHTML 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/content/html/content/src/nsGenericHTMLElement.cpp,
line 917]
nsGenericHTMLElementTearoff::SetInnerHTML 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/content/html/content/src/nsGenericHTMLElement.cpp,
line 213]
XPCWrappedNative::CallMethod 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/js/src/xpconnect/src/xpcwrappednative.cpp,
line 2065]
XPC_WN_GetterSetter 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/js/src/xpconnect/src/xpcwrappednativejsops.cpp,
line 1311]
js_Invoke 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/js/src/jsinterp.c, line 1320]
js_InternalInvoke 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/js/src/jsinterp.c, line 1417]
js_InternalGetOrSet 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/js/src/jsinterp.c, line 1460]
js_SetProperty 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/js/src/jsobj.c, line 2875]
js_Interpret 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/js/src/jsinterp.c, line 3448]
js_Invoke 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/js/src/jsinterp.c, line 1340]
js_InternalInvoke 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/js/src/jsinterp.c, line 1417]
JS_CallFunctionValue 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/js/src/jsapi.c, line 3851]
nsJSContext::CallEventHandler 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/dom/src/base/nsJSEnvironment.cpp,
line 1385]
nsJSEventListener::HandleEvent 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/dom/src/events/nsJSEventListener.cpp,
line 184]
nsEventListenerManager::HandleEventSubType 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/content/events/src/nsEventListenerManager.cpp,
line 1568]
nsEventListenerManager::HandleEvent 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/content/events/src/nsEventListenerManager.cpp,
line 1669]
nsGenericElement::HandleDOMEvent 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/content/base/src/nsGenericElement.cpp,
line 2132]
nsGlobalWindow::HandleDOMEvent 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/dom/src/base/nsGlobalWindow.cpp,
line 967]
DocumentViewerImpl::LoadComplete 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/layout/base/nsDocumentViewer.cpp,
line 992]
nsDocShell::EndPageLoad 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/docshell/base/nsDocShell.cpp,
line 4627]
nsWebShell::EndPageLoad 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/docshell/base/nsWebShell.cpp,
line 667]
nsDocShell::OnStateChange 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/docshell/base/nsDocShell.cpp,
line 4553]
nsDocLoader::FireOnStateChange 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/uriloader/base/nsDocLoader.cpp,
line 1194]
nsDocLoader::doStopDocumentLoad 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/uriloader/base/nsDocLoader.cpp,
line 832]
nsDocLoader::OnStopRequest 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/uriloader/base/nsDocLoader.cpp,
line 653]
nsLoadGroup::RemoveRequest 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/netwerk/base/src/nsLoadGroup.cpp,
line 732]
PresShell::RemoveDummyLayoutRequest 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/layout/base/nsPresShell.cpp,
line 7041]
PresShell::Destroy 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/layout/base/nsPresShell.cpp,
line 1993]
DocumentViewerImpl::Hide 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/layout/base/nsDocumentViewer.cpp,
line 1833]
nsSubDocumentFrame::Destroy 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/layout/generic/nsFrameFrame.cpp,
line 561]
nsBlockFrame::DoRemoveFrame 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/layout/generic/nsBlockFrame.cpp,
line 5597]
nsBlockFrame::RemoveFrame 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/layout/generic/nsBlockFrame.cpp,
line 5382]
nsFrameManager::RemoveFrame 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/layout/base/nsFrameManager.cpp,
line 708]
nsCSSFrameConstructor::ContentRemoved 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/layout/base/nsCSSFrameConstructor.cpp,
line 9816]
PresShell::ContentRemoved 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/layout/base/nsPresShell.cpp,
line 5487]
nsGenericElement::RemoveChild 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/content/base/src/nsGenericElement.cpp,
line 3278]
nsRange::DeleteContents 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/content/base/src/nsRange.cpp,
line 1592]
nsGenericHTMLElement::SetInnerHTML 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/content/html/content/src/nsGenericHTMLElement.cpp,
line 917]
nsGenericHTMLElementTearoff::SetInnerHTML 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/content/html/content/src/nsGenericHTMLElement.cpp,
line 213]
XPCWrappedNative::CallMethod 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/js/src/xpconnect/src/xpcwrappednative.cpp,
line 2065]
XPC_WN_GetterSetter 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/js/src/xpconnect/src/xpcwrappednativejsops.cpp,
line 1311]
js_Invoke 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/js/src/jsinterp.c, line 1320]
js_InternalInvoke 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/js/src/jsinterp.c, line 1417]
js_InternalGetOrSet 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/js/src/jsinterp.c, line 1460]
js_SetProperty 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/js/src/jsobj.c, line 2875]
js_Interpret 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/js/src/jsinterp.c, line 3448]
js_Invoke 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/js/src/jsinterp.c, line 1340]
js_InternalInvoke 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/js/src/jsinterp.c, line 1417]
JS_CallFunctionValue 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/js/src/jsapi.c, line 3851]
nsJSContext::CallEventHandler 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/dom/src/base/nsJSEnvironment.cpp,
line 1385]
nsJSEventListener::HandleEvent 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/dom/src/events/nsJSEventListener.cpp,
line 184]
nsEventListenerManager::HandleEventSubType 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/content/events/src/nsEventListenerManager.cpp,
line 1568]
nsEventListenerManager::HandleEvent 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/content/events/src/nsEventListenerManager.cpp,
line 1669]
nsGenericElement::HandleDOMEvent 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/content/base/src/nsGenericElement.cpp,
line 2132]
nsGlobalWindow::HandleDOMEvent 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/dom/src/base/nsGlobalWindow.cpp,
line 967]
DocumentViewerImpl::LoadComplete 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/layout/base/nsDocumentViewer.cpp,
line 992]
nsDocShell::EndPageLoad 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/docshell/base/nsDocShell.cpp,
line 4627]
nsWebShell::EndPageLoad 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/docshell/base/nsWebShell.cpp,
line 667]
nsDocShell::OnStateChange 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/docshell/base/nsDocShell.cpp,
line 4553]
nsDocLoader::FireOnStateChange 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/uriloader/base/nsDocLoader.cpp,
line 1194]
nsDocLoader::doStopDocumentLoad 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/uriloader/base/nsDocLoader.cpp,
line 832]
nsDocLoader::OnStopRequest 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/uriloader/base/nsDocLoader.cpp,
line 653]
nsLoadGroup::RemoveRequest 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Depend/mozilla/netwerk/base/src/nsLoadGroup.cpp,
line 732]
URL: www.asachi.com/ff_crash.html → http://www.asachi.com/ff_crash.html
Keywords: crash, testcase
Attached file Html with the bug reproducer (obsolete) — 
On pressing "Crash Me" firefox will crash.. Or try multiple times if not.. Also
create a blank.html and put there somethink
(In reply to comment #1)
> reporter: please run components\talkback and copy the incident id.

TB5686559Q
Status: UNCONFIRMED → NEW
Ever confirmed: true
Summary: Overwriting of div innerHTML cause starting of the loading icon(circle) and some times browser can crash → Overwriting of div innerHTML cause starting of the loading icon(circle) and some times browser can crash [@ nsRange::DeleteContents]
Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8b2) Gecko/20050509
Firefox/1.0+

FWIW I don't get a crash on my debug build. Perhaps it is platform specific, or
I have some spackle stopping the crash.
Attached patch Patch (obsolete) — — Splinter Review 

    
    

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

This patch doesn't work, maybe if (!parent) break; works (it doesn't crash),
but i'm not sure if the testcase behaves as it should: It only switches once
between div and iframe (i expected 12 because of for(i = 1; i <= 12; i++)
makeIframe();)?

        Attachment #183074 -
        Attachment is obsolete: true

    
    

    
  
Can someone create a minimized testcase?  Is the loop over 12 things needed?
Note that the page, as written, should end up going into an infinite-ish loop,
with a more and more deeply nested DOM.  Is that what's causing problems?

    
    

    
  

      
      
      
      

        Attached file
          
        testcase
        — 
      

    


  
This crashes consistently for me when clicking on the button.
This doesn't crash in Mozilla1.0, but there it does the wrong thing: it doesn't
delete the iframes.
In Mozilla1.1 it starts to crash witht the testcase.

        Attachment #183012 -
        Attachment is obsolete: true

    
    

    
  
The problem is that setting innerHTML removes the iframe, which triggers its
onload, which reenters SetInnerHTML.  Similarly, just reentering
nsRange::DeleteContents from an event triggered by nsRange::DeleteContents crashes.

The problem seems to be that we end up with a |node| whose parent is already
null, so when we go to call RemoveChild on the parent, we crash.
Assignee: general → traversal-range
Component: DOM: HTML → DOM: Traversal-Range
OS: Windows XP → All
Hardware: PC → All
Summary: Overwriting of div innerHTML cause starting of the loading icon(circle) and some times browser can crash [@ nsRange::DeleteContents] → Reentering nsRange::DeleteContents for same-ish range crashes

    
    

    
  
So the general problem is that the iterator used by nsRange::DeleteContents
(nsSubtreeIterator, in the end), doesn't really deal with document mutations. 
It will happily return a node that's no longer in the document, which is what
happens here.  It's not clear to me what the correct behavior is for DOM events
rearranging the DOM the range is trying to delete while it's trying to delete
it, so I'm tempted to just null-check here and be done with it.  sicking? 
peterv?  jst?  Thoughts?

    
    

    
  
Actually, a null-check won't work.  The iterator holds a _weak_ ref to the
content.  So in my testcase it'll actually end up returning an already-deleted node.

    
    

    
  

      
      
      
      

        Attached patch
          
          
        Proposed patch
        — 
          — Splinter Review
      

    


  
Well, I was wrong.  The iterator holds a strong ref.  But generic element was
failing to deal with RemoveChildAt rearranging the DOM...  With this patch,
both testcases are happy.

        Attachment #188611 -
        Flags: superreview?(peterv)

        Attachment #188611 -
        Flags: review?(bugmail)

    
    

    
  
Comment on attachment 188611 [details] [diff] [review]
Proposed patch

>Index: content/base/src/nsGenericElement.cpp
>===================================================================

>@@ -2766,29 +2766,33 @@ nsGenericElement::RemoveChildAt(PRUint32
> 
>     if (HasMutationListeners(this, NS_EVENT_BITS_MUTATION_NODEREMOVED)) {
>       nsMutationEvent mutation(PR_TRUE, NS_MUTATION_NODEREMOVED, oldKid);
>       mutation.mRelatedNode = do_QueryInterface(this);
> 
>       nsEventStatus status = nsEventStatus_eIgnore;
>       oldKid->HandleDOMEvent(nsnull, &mutation, nsnull,
>                              NS_EVENT_FLAG_INIT, &status);
>     }
> 
>-    nsRange::OwnerChildRemoved(this, aIndex, oldKid);
>+    // Someone may have removed the kid while that event was processing...
>+    if (oldKid->GetParent() == this && aIndex < GetChildCount() &&
>+        oldKid == GetChildAt(aIndex)) {

We could do this check only in the case where there's a mutation listener?

        Attachment #188611 -
        Flags: superreview?(peterv) → superreview+

    
    

    
  
Good idea.  I'll make that change.

    
    

    
  
Comment on attachment 188611 [details] [diff] [review]
Proposed patch

I don't quite understand how this is reentering the range code. 

>+    // Someone may have removed the kid while that event was processing...
>+    if (oldKid->GetParent() == this && aIndex < GetChildCount() &&
>+        oldKid == GetChildAt(aIndex)) {

No need to bounds-check aIndex, GetChildAt will return null so that test will
always fail if aIndex is too big.

r=me

        Attachment #188611 -
        Flags: review?(bugmail) → review+

    
    

    
  
Comment on attachment 188611 [details] [diff] [review]
Proposed patch

Requesting approval for some sanity-checking to deal with the DOM mutating
under us...

        Attachment #188611 -
        Flags: approval1.8b4?

    
    

    
  
> I don't quite understand how this is reentering the range code. 

Which?  "testcase", or "testcase that makes it clearer..."?  They reenter via
different paths, of course...

    
  

        Attachment #188611 -
        Flags: approval1.8b4? → approval1.8b4+

    
  
Assignee: traversal-range → bzbarsky
Priority: -- → P1
Target Milestone: --- → mozilla1.8beta4

    
    

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

    
    

    
  
Verified FIXED using https://bugzilla.mozilla.org/attachment.cgi?id=188606 and
https://bugzilla.mozilla.org/attachment.cgi?id=186071 as testcases with build
2005-07-27-05 under Windows XP SeaMonkey trunk--no crash.
Status: RESOLVED → VERIFIED

    
    

    
  
content/base/crashtests/293388-1.html
http://hg.mozilla.org/mozilla-central/rev/b0337b6287f3
Flags: in-testsuite+
Component: DOM: Traversal-Range → 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: