[webkit-reviews] review granted: [Bug 19933] nodeIterator with filter fails on documents not in a frame : [Attachment 22145] Revised patch addressing comments

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Simon Fraser
<simon.fraser at apple.com>'s request for review:
Bug 19933: nodeIterator with filter fails on documents not in a frame
https://bugs.webkit.org/show_bug.cgi?id=19933

Attachment 22145: Revised patch addressing comments
https://bugs.webkit.org/attachment.cgi?id=22145&action=edit

------- Additional Comments from Darin Adler <darin at apple.com>
     /* exec should only be null when calling from a non-JavaScript language to

	a JavaScript filter, when the document in question is not associated
	with a frame
     */

We normally use "//" comments, not "/*". I also would prefer that the comment
be a sentence with a capital letter and period. Here's my suggested wording:

    // The exec argument here should only be null if this was called from a
    // non-JavaScript language, and this is a JavaScript filter, and the
document
    // in question is not associated with the frame. In that case, we're going
to
    // behave incorrectly, and just reject nodes instead of calling the filter
function.
    // To fix that we'd need to come up with a way to find a suitable
JavaScript
    // execution context for the filter function to run in.

I know that's pretty wordy, sorry.

    /* static */
    ExecState* NodeFilter::execStateFromNode(Node* node)

We don't normally add a comment like this before a static member function.

r=me