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

[bugzilla-daemon at webkit.org]

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





--- Comment #9 from Dai Mikurube <dmikurube at google.com>  2010-12-03 03:46:23 PST ---
(From update of attachment 75465)
View in context: https://bugs.webkit.org/attachment.cgi?id=75465&action=review

For tests, later.

>> WebCore/html/HTMLSelectElement.cpp:115
>> +    if (!isRequiredFormControl() || disabled() || multiple() || size() > 1)
> 
> We don't need to check disabled() here.  It's responsibility of willValidate().
> 
> We should not return false for multiple() unconditionally.  The specification says we never have placeholder label option for multiple, but required should work with multiple.

Thanks, the code is wrong. Revised.

>> WebCore/html/HTMLSelectElement.cpp:297
>> +        setNeedsValidityCheck();
> 
> Why is this needed?

I misunderstood the role of appendFormData(). It looks better to change the first argument, data, of appendForData to const.

>> WebCore/html/HTMLSelectElement.cpp:417
>> +    setNeedsValidityCheck();
> 
> We don't need this.  setAttribute() calls parseMappedAttribute().

Thanks. Removed.

>> WebCore/html/HTMLSelectElement.cpp:502
>> +    setNeedsValidityCheck();
> 
> Why is this needed?

Exactly. Removed. It looks better to change data to const, too.

>> WebCore/html/HTMLSelectElement.h:48
>> +    bool isValidValue(const String& candidate) const { return !valueMissing(candidate); }
> 
> isValidValue() is not used.  We don't need to add it.
> So, the parameter of valueMissing() is not needed.  It's always this->value().

Removed. It followed HTMLInputElement and HTMLTextAreaElement. Isn't it required? It looks a kind of inconsistent.
HTMLTextAreaElement::isValidValue() looks not called. (It is called from WebKit/chromium as for HTMLInputElement.)

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