[webkit-reviews] review granted: [Bug 27959] Support for validationMessage : [Attachment 43327] Patch v2c, corrected

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Michelangelo De Simone
<micdesim at gmail.com>'s request for review:
Bug 27959: Support for validationMessage
https://bugs.webkit.org/show_bug.cgi?id=27959

Attachment 43327: Patch v2c, corrected
https://bugs.webkit.org/attachment.cgi?id=43327&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
> +    if (customError())
> +	   return m_customErrorMessage;
> +    else if (valueMissing())
> +	   return validationMessageValueMissingText();
> +    else if (typeMismatch())
> +	   return validationMessageTypeMismatchText();
> +    else if (patternMismatch())
> +	   return validationMessagePatternMismatchText();
> +    else if (tooLong())
> +	   return validationMessageTooLongText();
> +    else if (rangeUnderflow())
> +	   return validationMessageRangeUnderflowText();
> +    else if (rangeOverflow())
> +	   return validationMessageRangeOverflowText();
> +    else if (stepMismatch())
> +	   return validationMessageStepMismatchText();

Normally we don't do else after return.

> +    return emptyAtom;

emptyAtom is an AtomicString and we just need a String here. On the other hand,
emptyAtom might be a relatively efficient way to return the empty string, so
I'm not sure this is a bad thing!

It seems to me that returning a non-localized hard-wired string before calling
notImplemented() would probably be better for all those ports where this is not
done yet.

This patch sure does seem to be an eloquent argument for not having all these
localized strings in the HTML5 form element design! I don't see a lot of value
of returning these strings that might not even be in the same language as the
website. I wish we could get that changed.

r=me