Closed
Bug 10231
Opened 25 years ago
Closed 25 years ago
crash making list after select all
Categories
(Core :: DOM: Editor, defect, P3)
Core
DOM: Editor
Tracking
()
VERIFIED
FIXED
M9
People
(Reporter: mozeditor, Assigned: kinmoz)
Details
Open default editor page. Select All. Make a List (either kind). Crasheroonies.
Reporter | ||
Updated•25 years ago
|
Status: NEW → ASSIGNED
Target Milestone: M9
Reporter | ||
Comment 1•25 years ago
|
||
accepting, setting to m9.
Reporter | ||
Updated•25 years ago
|
Summary: crash making list after select all
Reporter | ||
Comment 2•25 years ago
|
||
setting summary
I have a fix for this. There were actually 2 problems here. 1. nsHTMLEditRules::GetPromotedPoint() didn't account for the fact that the GetChildAt() call could return NULL for the case where aWhere == kEnd. 2. nsGenericElement::RangeAdd() did not check to see if a range was already in it's list, so if a range had a start and end point in the same parent, that range would be added twice, and notified twice of any DOM tree changes. Here are the diffs for the fixes: Index: nsHTMLEditRules.cpp =================================================================== RCS file: /cvsroot/mozilla/editor/base/nsHTMLEditRules.cpp,v retrieving revision 1.37 diff -c -r1.37 nsHTMLEditRules.cpp *** nsHTMLEditRules.cpp 1999/07/26 18:05:41 1.37 --- nsHTMLEditRules.cpp 1999/08/02 18:46:58 *************** *** 1265,1271 **** { node = nsEditor::GetChildAt(parent,offset); } ! offset++; // since this is going to be used for a range _endpoint_, we wa nt to be after the node // finding the real end for this point. look up the tree for as long as w e are the // last node in the container, and as long as we haven't hit the body node . --- 1265,1275 ---- { node = nsEditor::GetChildAt(parent,offset); } ! ! if (node) ! offset++; // since this is going to be used for a range _endpoint_, we want to be after the node ! else ! node = parent; // finding the real end for this point. look up the tree for as long as we are the // last node in the container, and as long as we haven't hit the body node. Index: nsGenericElement.cpp =================================================================== RCS file: /cvsroot/mozilla/layout/base/src/nsGenericElement.cpp,v retrieving revision 3.46 diff -c -r3.46 nsGenericElement.cpp *** nsGenericElement.cpp 1999/07/22 22:34:28 3.46 --- nsGenericElement.cpp 1999/08/02 18:58:16 *************** *** 821,826 **** --- 821,836 ---- if (nsnull == mDOMSlots->mRangeList) { return NS_ERROR_OUT_OF_MEMORY; } + + // Make sure we don't add a range that is already + // in the list! + PRInt32 i = mDOMSlots->mRangeList->IndexOf(&aRange); + if (i >= 0) { + // Range is already in the list, so there + // is nothing to do! + return NS_OK; + } + // dont need to addref - this call is made by the range object itself PRBool rv = mDOMSlots->mRangeList->AppendElement(&aRange); if (rv) return NS_OK;
One more fix to make this work properly. nsDOMSelection::GetPrimaryFrameForFocusNode() did not null out it's return parameter, so if it failed to get a focus node, it might still return NS_OK and a pointer that was pointing to lala-land. Index: nsRangeList.cpp =================================================================== RCS file: /cvsroot/mozilla/layout/base/src/nsRangeList.cpp,v retrieving revision 1.117 diff -c -r1.117 nsRangeList.cpp *** nsRangeList.cpp 1999/07/22 19:42:21 1.117 --- nsRangeList.cpp 1999/08/02 20:27:04 *************** *** 1505,1510 **** --- 1505,1512 ---- if (!aReturnFrame) return NS_ERROR_NULL_POINTER; + *aReturnFrame = 0; + nsresult result = NS_OK; nsCOMPtr<nsIDOMNode> node = dont_QueryInterface(FetchFocusNode()); *************** *** 1525,1532 **** { nsCOMPtr<nsIContent> child; result = content->ChildAt(offset, *getter_AddRefs(child)); ! if (NS_FAILED(result) || !child) //out of bounds? return result; content = child;//releases the focusnode } } --- 1527,1536 ---- { nsCOMPtr<nsIContent> child; result = content->ChildAt(offset, *getter_AddRefs(child)); ! if (NS_FAILED(result)) return result; + if (!child) //out of bounds? + return NS_ERROR_FAILURE; content = child;//releases the focusnode } }
Reassigning bug to kin@netscape.com.
Status: ASSIGNED → RESOLVED
Closed: 25 years ago
Resolution: --- → FIXED
Fix checked in: mozilla/editor/base/nsHTMLEditRules.cpp rev 1.38 - Added check, in WillDeleteSelection(), to see if endpoints of the range are in the body before calling GetBlockNodeParent(). mozilla/layout/base/src/nsGenericElement.cpp rev 3.47 - Modified RangeAdd() so that it doesn't add a range if it's already in the list. mozilla/layout/base/src/nsRangeList.cpp rev 1.118 - Modified GetPrimaryFrameForFocusNode() to initialize aReturnFrame and to return a failure if ChildAt() returns NULL.
Reporter | ||
Comment 8•25 years ago
|
||
just a note that I had to make some more changes. I added Kin's nsGenericElement::RangeAdd() fix to nsGenericDOMDataNode::RangeAdd(), and also modified nsRange::DoSetRange() to work with the new dom gravity rangelist assumption of "range will be on list once or not at all", whereas before it could be on 0, 1, or 2 times.
Comment hidden (collapsed) |
You need to log in
before you can comment on or make changes to this bug.
Description
•