[Webkit-unassigned] [Bug 50380] Implement "required" attribute for select tags

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=50380


Kent Tamura <tkent at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #75788|review?                     |review-
               Flag|                            |




--- Comment #24 from Kent Tamura <tkent at chromium.org>  2010-12-07 01:55:44 PST ---
(From update of attachment 75788)
View in context: https://bugs.webkit.org/attachment.cgi?id=75788&action=review

Please rebase.  Unfortunately, another patch which conflicts with this has been committed.

> WebCore/html/HTMLSelectElement.cpp:134
> +    // If nothing is selected, it's value-missing.

Translating simple code to English is not helpful.  Please remove the comment.
We usually add a comment which describes a reason of unclear code.  So the above comment about size() is good.

> WebCore/html/HTMLSelectElement.cpp:137
> +    // If a non-placeholer label option is selected, it's not value-missing.

This is a good comment.  It explains why 0 is not included.

> WebCore/html/HTMLSelectElement.cpp:142
> +    // It may be a placeholder label option.
> +    // Check if the option is not disabled and not under optgroup elements, and the value is the empty string.

This comment is not good because it doesn't explain the reason of the code, but the code is unclear for one who doesn't know "placeholder label option" concept.
So, we should introduce a new function, hasPlaceholderLabelOption(), and should remove the comment.

> WebCore/html/HTMLSelectElement.cpp:206
> -String HTMLSelectElement::value()
> +String HTMLSelectElement::value() const

This change is not related to this bug.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.