[webkit-reviews] review denied: [Bug 31428] getElementsByTagName("div")['elementId'] lookup fails if two elements have the same id : [Attachment 43091] fix patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Nov 25 08:53:21 PST 2009


Eric Seidel <eric at webkit.org> has denied Chang Shu <Chang.Shu at nokia.com>'s
request for review:
Bug 31428: getElementsByTagName("div")['elementId'] lookup fails if two
elements have the same id
https://bugs.webkit.org/show_bug.cgi?id=31428

Attachment 43091: fix patch
https://bugs.webkit.org/attachment.cgi?id=43091&action=review

------- Additional Comments from Eric Seidel <eric at webkit.org>
cshu and I spoke over IRC.  The fix looks correct, but we need to better
communicate here why it's correct.

1.  The test case needs to be cleaned up to use better variable naming, and
probably just shorter in general.  We could consider converting it to a normal
"script-test", although that's not required. 
http://trac.webkit.org/wiki/Writing%20Layout%20Tests%20for%20DumpRenderTree

2.  We need information on why this change is correct.	Does this new behavior
conform to a spec?  Does this new behavior match FF/IE?  If the answer to
either of those questions is "no" we need to know why.

Basically we just need to make this patch super-easy to understand, and thus
review.  Once that's done, I think this will be a very quick r+.


More information about the webkit-reviews mailing list