[webkit-reviews] review granted: [Bug 66321] Handle "form" attribute updates in parseMappedAttribute() instead of attributeChanged() to better match HTMLElement practices : [Attachment 104072] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Adam Klein <adamk at chromium.org>'s
request for review:
Bug 66321: Handle "form" attribute updates in parseMappedAttribute() instead of
attributeChanged() to better match HTMLElement practices
https://bugs.webkit.org/show_bug.cgi?id=66321

Attachment 104072: Patch
https://bugs.webkit.org/attachment.cgi?id=104072&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=104072&action=review


Do we have test coverage for this? I suggest removing the three things done by
attributeChanged one at a time and verifying that in each case an existing
regression test fails. If not, then we should add regression tests. Since the
benefit level of this is low (makes the code slightly more consistent and
elegant) we want to be doubly sure we are not breaking anything.

I’m going to say review+ but I do not think this should go in unless we are
sure we have test coverage.

> Source/WebCore/html/HTMLObjectElement.cpp:90
> +    if (attr->name() == formAttr) {
> +	   formAttributeChanged();
> +    } else if (attr->name() == typeAttr) {

WebKit coding style says to not use braces around a single-line body like this
one.