[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
http://bugs.webkit.org/show_bug.cgi?id=14955
Attachment 17870: updated patch
http://bugs.webkit.org/attachment.cgi?id=17870&action=edit
------- 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