[webkit-reviews] review denied: [Bug 27455] Support custom validity error message : [Attachment 34195] Proposed patch, rev. 3a

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Aug 5 18:09:53 PDT 2009


Darin Adler <darin at apple.com> has denied Michelangelo De Simone
<micdesim at gmail.com>'s request for review:
Bug 27455: Support custom validity error message
https://bugs.webkit.org/show_bug.cgi?id=27455

Attachment 34195: Proposed patch, rev. 3a
https://bugs.webkit.org/attachment.cgi?id=34195&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
>  ValidityState::ValidityState(HTMLFormControlElement* parent)
>      : m_control(parent)
> +    , m_customErrorMessage()
>  {
>      ASSERT(parent);
>  }

No change needed here. This will happen without you explicitly listing the name
of the data member since it's not a plain type like int, so you should leave
this out.

> +	   void setCustomErrorMessage(const String& e) { m_customErrorMessage =
e; }

We normally use short words rather than single letters for this sort of thing.
It should be "message" instead of "e".

> -	   bool customError() { return false; }
> +	   bool customError() { return !m_customErrorMessage.isEmpty() &&
!m_customErrorMessage.isNull(); }

Since all null messages are empty there is no need for the separate isNull
check here.

This is looking good, but please fix these.


More information about the webkit-reviews mailing list