Closed Bug 304514 Opened 19 years ago Closed 3 years ago

nsContentIterator::Init(range) doesn't consider Document nodes


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

Windows XP





(Reporter: WeirdAl, Unassigned)




(Keywords: assertion, hang, testcase)


(1 file)

Derived from bug 157373.  A patch there fixes nsRange::ToString() for document
nodes, but in the process a similar bug in nsContentIterator causes an endless
loop with assertions.

###!!! ASSERTION: Null current node in an iterator that's not done!: 'mCurNode',
file c:/mozsource/mozilla/content/base/src/nsContentIterator.cpp, line 1191

nsContentIterator::Init() sets mCommonParent = do_QueryInterface(dN), where
mCommonParent is of type nsIContent and dN is a nsIDOMNode.  However, dN can be
a Document node, in which case the QI will return null.

Similar problems occur with StartCon a few lines later, resulting in the Init
function bailing out and returning NS_ERROR_FAILURE.  Naturally, the next frame
in the stack, nsRange::ToString(), didn't bother checking a return value:


A similar bug exists for EndCon, a few lines later.  I don't know whether or not
other failures exist here.

Steps to reproduce:
(1) Apply attachment 192565 [details] [diff] [review] to your tree.
(2) make tier_9
(3) Start your browser and point it to bug 157373.
(4) Click on the URL link in the bug.

Expected results:
Two alerts.

Actual results:
One alert, followed by the above assertion in an endless loop, and hang.
Blocks: 157373
Attached patch work-in-progressSplinter Review
Here's what I have after a little thinking:

// nsContentIterator::Init(nsIDOMRange* aRange) has to set these.
  nsCOMPtr<nsIContent> mCurNode;
  nsCOMPtr<nsIContent> mFirst;
  nsCOMPtr<nsIContent> mLast;
  nsCOMPtr<nsIContent> mCommonParent;

 * Flag for nsPreContentIterator.  
  PRBool mPre;

 * This array is cleared in Init(), and then populated in RebuildIndexStack.
  nsAutoVoidArray mIndexes;

// Not changed in Init.
  PRInt32 mCachedIndex;

// False, except at end on Init, where it becomes !mCurNode.

This patch attempts to set mCurNode, mFirst and mLast accordingly when we have
a boundary point in the range as a document.

Unfortunately, mCurrentParent is still used quite heavily throughout the rest
of the iterator, and it will be null under these circumstances.  I don't have
any way of knowing how to fix the code for this.  RebuildIndexStack, for
instance, will eventually return NS_ERROR_FAILURE.  I shudder at the thought of
GetNextSibling, NextNode and friends.
> // False, except at end on Init, where it becomes !mCurNode.
Assignee: general → nobody
QA Contact: ian → general

Move all DOM bugs that haven’t been updated in more than 3 years and has no one currently assigned to P5.

If you have questions, please contact :mdaly.
Priority: -- → P5
Component: DOM → DOM: Core & HTML

Hey Alex,
Is this issue still occurring for you or can we close it?

Flags: needinfo?(ajvincent)

I'd say close it. I have no idea what the state of the code is, but I haven't touched this in over a decade.

Flags: needinfo?(ajvincent)

Marking this as Resolved > Worksforme based on the last comment.

Closed: 3 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.