719101 - Generic nsFind Cleanup

Status: RESOLVED WORKSFORME

(Core :: Find Backend, defect)

Description

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Attachment: Patch

Comment 1

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

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

Comment 2

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Looks like hsivonen did this.