Comment on attachment 589536 [details] [diff] [review] Patch Review of attachment 589536 [details] [diff] [review]: ----------------------------------------------------------------- I disclaim any responsibility for this code. Bonus points if you make GetBlockParent work on nsIContent or nsINode and not return nsresult. ::: embedding/components/find/src/nsFind.cpp @@ +453,5 @@ > nsFindContentIterator* it = new nsFindContentIterator(aFindBackward); > if (!it) { > return NS_ERROR_OUT_OF_MEMORY; > } > return it->QueryInterface(NS_GET_IID(nsIContentIterator), (void **)aResult); Looks like this is dead code, please remove it. @@ +736,1 @@ > bool nsFind::IsBlockNode(nsIContent* aContent) Yeah, because we really need another IsBlockNode... @@ +748,3 @@ > return true; > > + bool isBlock = false; Move this back to where it was @@ +754,3 @@ > } > > + parserService->IsBlock(parserService->HTMLAtomTagToId(atom), isBlock); Can we move this somewhere maintained already? @@ +757,4 @@ > return isBlock; > } > > bool nsFind::IsTextNode(nsIDOMNode* aNode) Function looks unused too @@ +789,5 @@ > atom = aContent->Tag(); > > // We may not need to skip comment nodes, > // now that IsTextNode distinguishes them from real text nodes. > return (aContent->IsNodeOfType(nsINode::eCOMMENT) || Make this aContent->NodeType() == nsIDOMNode::COMMENT_NODE @@ +798,3 @@ > > #else /* HAVE_BIDI_ITERATOR */ > // Temporary: eventually we will have an iterator to do this, Ha. Ha. Ha. @@ +803,5 @@ > // and take the performance hit. > > nsIContent *content = aContent; > while (content) > { Turn this into a for loop. @@ +806,5 @@ > while (content) > { > atom = content->Tag(); > > if (aContent->IsNodeOfType(nsINode::eCOMMENT) || And here ::: embedding/components/find/src/nsFind.h @@ +78,3 @@ > > PRInt32 mIterOffset; > nsCOMPtr<nsIDOMNode> mIterNode; I wonder if it's worthwhile to reorder these to pack better

Looks like hsivonen did this.