[webkit-reviews] review granted: [Bug 137959] Assertion hit in DocumentOrderedMap::get while removing a form element : [Attachment 329467] Fixes the assertion

[bugzilla-daemon at webkit.org]

Brent Fulgham <bfulgham at webkit.org> has granted Ryosuke Niwa
<rniwa at webkit.org>'s request for review:
Bug 137959: Assertion hit in DocumentOrderedMap::get while removing a form
element
https://bugs.webkit.org/show_bug.cgi?id=137959

Attachment 329467: Fixes the assertion

https://bugs.webkit.org/attachment.cgi?id=329467&action=review




--- Comment #9 from Brent Fulgham <bfulgham at webkit.org> ---
Comment on attachment 329467
  --> https://bugs.webkit.org/attachment.cgi?id=329467
Fixes the assertion

View in context: https://bugs.webkit.org/attachment.cgi?id=329467&action=review

Looks good! I assume the original assertion is still worth having around --
you've just silenced instances of the assertion firing that were not really
error conditions, right?

> Source/WebCore/ChangeLog:11
> +	   attribute of the form associated element. When there are other
elements with the same ID in the removed tree,

I feel like this first sentence is missing something. What happens with the
very first element with the ID of the form content attribute? Is this just
missing "...form associated element was removed. When there are ..."?

> Source/WebCore/dom/ContainerNodeAlgorithms.h:29
> +// FIXME: Delete this class after fixing FormAssociatedElement to avoid
calling getElementById during a tree removal.

Should this be a Bugzilla with a number in the FIXME?