[webkit-reviews] review denied: [Bug 19079] Send the submissions character encoding in hidden _charset_ field : [Attachment 113639] Updated Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has denied Vineet Chaudhary (vineetc)
<rgf748 at motorola.com>'s request for review:
Bug 19079: Send the submissions character encoding in hidden _charset_ field
https://bugs.webkit.org/show_bug.cgi?id=19079

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

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


Seems fine, but implementation is not quite right.

> Source/WebCore/html/InputType.cpp:178
> +    if (equalIgnoringCase(element()->name(), "_charset_") &&
element()->isInputTypeHidden())

This is the wrong way to code something that’s type specific in InputType
objects. Instead, HiddenInputType should override the appendFormData function.

The InputType objects were created to avoid having type-specific code mixed in
with type-independent code, making use of isXXX checks like the one here.

> Source/WebCore/html/InputType.cpp:179
> +	   encoding.appendData(element()->name(),
String(encoding.encoding().name()));

Why is an explicit String() needed here? Did you try compiling without it?