187414 - Use NodeFilters for DOM View

Status: RESOLVED FIXED

(Other Applications :: DOM Inspector, defect)

Description

[Christopher Aillon (sabbatical, not receiving bugmail)]

Comment 1

[Christopher Aillon (sabbatical, not receiving bugmail)]

Attachment: Why didn't it use NodeFilter before?

Comment 2

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 110476 [details] [diff] [review]
Why didn't it use NodeFilter before?

> +    PRUint32 filterForNodeType = 1 << (nodeType - 1);

Ew.  This assumes all sorts of stuff about the filter consts and the node
types...

Perhaps an inline function to do this would be in order; one which asserts that
|nodeType| is in a certain range (that standardized by the existing DOM3 spec?)

At the very least, this line deserves a nice comment explaining what's going
on.

Other than that, looks good.

Comment 3

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 110476 [details] [diff] [review]
Why didn't it use NodeFilter before?

>+    PRUint32 filterForNodeType = 1 << (nodeType - 1);

What bz said. Please add checks to make sure that nodeType is within the
allowed range (1-12).


>Index: extensions/inspector/resources/content/viewers/domNode/domNode.js
>===================================================================
>@@ -71,7 +71,7 @@
> 
>   // prepare and attach the DOM DataSource
>   this.mDOMView = XPCU.createInstance(kDOMViewCID, "inIDOMView");
>-  this.mDOMView.addFilterByType(2, true);
>+  this.mDOMView.whatToShow |= NodeFilter.SHOW_ATTRIBUTE;

Shouldn't this be 
this.mDOMView.whatToShow = NodeFilter.SHOW_ATTRIBUTE;

since the second argument was |true|.

with that, r=sicking

Comment 4

[Christopher Aillon (sabbatical, not receiving bugmail)]

Checked in.