[webkit-reviews] review denied: [Bug 14955] Implement getElementsByClassName : [Attachment 17870] updated patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Dec 12 17:55:11 PST 2007

Darin Adler <darin at apple.com> has denied Sam Weinig <sam at webkit.org>'s request
for review:
Bug 14955: Implement getElementsByClassName

Attachment 17870: updated patch

------- Additional Comments from Darin Adler <darin at apple.com>
There's an extra change log entry here, for bug 15313.

I really wish we didn't use a list for ClassNameList. A Vector<AtomicString>*
would work better, I think. It's really bad that deleting a ClassNameList uses
O(n) stack in the destructor.

Is it really good to indent ChildNodeList.h as part of this patch? There's also
an unneeded forward declaration added to that file. Please land that separately
if at all.

We should use foldCase() instead of lower() for contexts like this one.

The check of isLower() in parseClassAttribute is unnecessary. String::lower()
should already do that.

 41	, m_namespaceURI(nullAtom)

Isn't nullAtom the default anyway?

Otherwise looks good to me. I'll say review- for now because I want you to
consider the comments above.

More information about the webkit-reviews mailing list