[webkit-reviews] review granted: [Bug 44879] Setting form.enctype reflected attribute behaves strangely : [Attachment 111414] Updated Patch

[bugzilla-daemon at webkit.org]

Alexey Proskuryakov <ap at webkit.org> has granted Vineet Chaudhary (vineetc)
<rgf748 at motorola.com>'s request for review:
Bug 44879: Setting form.enctype reflected attribute behaves strangely
https://bugs.webkit.org/show_bug.cgi?id=44879

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

------- Additional Comments from Alexey Proskuryakov <ap at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=111414&action=review


Revision 4 is initial KHTML import, and following the history further than that
is rarely useful.

This change looks good, r=me.

> LayoutTests/fast/forms/enctype-attribute.html:27
> +shouldBe('form1.enctype = "text"; form1.enctype',
'"application/x-www-form-urlencoded"');
> +shouldBe('form1.enctype = "plain"; form1.enctype',
'"application/x-www-form-urlencoded"');
> +shouldBe('form1.enctype = "multipart"; form1.enctype',
'"application/x-www-form-urlencoded"');
> +shouldBe('form1.enctype = "form-data"; form1.enctype',
'"application/x-www-form-urlencoded"');

Please add subtests for proper MIME types, and verify that they also pass in
Firefox. E.g. multipart/mixed, multipart/digest, text/css.

Whatever the reason for the existing code was, it was likely not to support
"text", but to support other "text/*" subtypes etc.

> LayoutTests/fast/forms/enctype-attribute.html:39
> +debug('Webkit should not allow leading whitespace.');

This message is slightly misleading. There is nothing special about WebKit - no
major engine trims whitespace when parsing enctype attribute. Besides, it's not
only leading, but also trailing whitespace (and the latter should also be
tested here).