[webkit-reviews] review requested: [Bug 9508] JavaScript Comparing a form element group to itself returns false. : [Attachment 12621] proposed fix

[bugzilla-request-daemon at macosforge.org]

Sanjay Madhav (chmmravatar) <sanjay12 at gmail.com> has asked  for review:
Bug 9508: JavaScript Comparing a form element group to itself returns false.
http://bugs.webkit.org/show_bug.cgi?id=9508

Attachment 12621: proposed fix
http://bugs.webkit.org/attachment.cgi?id=12621&action=edit

------- Additional Comments from Sanjay Madhav (chmmravatar)
<sanjay12 at gmail.com>
Weighing the options, the posted patch is probably the best way to solve the
issue at hand given all the factors.

While it certainly would be possible to cache requests for form groups in the
HTMLFormElement, the issue becomes that if we don't cache every single unique
request for the life of the HTMLFormElement, then you potentially could have a
comparison occur much later that fails when it shouldn't. However, having a
cache which only removes elements once the HTMLFormElement gets destroyed would
add memory overhead. The comments surrounding DOMNameNodesCollection seemed to
imply that it would be too costly to have them persist for long periods of
time.

We could change how form elements are stored internally, so the group lookup
doesn't have to create a temporary object, but it would probably end up
touching a great deal of code.

With the solution used in the patch, for 99% of the equality comparisons,
there'll be no discernable speed effect, and it also gives some flexibility to
quickly fix similar problems in other areas, if they pop up.

I tested this change with both the LayoutTests and javascriptcore tests, and no
regressions are introduced by this patch.

Also included is a test case for future regression testing.