[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 #17 from Dai Mikurube <dmikurube at google.com>  2010-12-05 22:16:00 PST ---
(From update of attachment 75494)
View in context: https://bugs.webkit.org/attachment.cgi?id=75494&action=review

>> LayoutTests/fast/forms/ValidityState-valueMissing-001-expected.txt:2
>>  
> 
> The purpose of ValidityState-valueMissing-001.html was test cases for validity.valueMissing==true.
> If you follow it, you should remove cases with valueMissing==false.

I considered it as test cases for validity.valueMissing when blank values are given.
"required" attributes are common in these elements, though they may have different results in some conditions.

So, IFMO, collecting cases by similar conditions (e.g. with empty values / with some non-empty values) may be convenient than separating them by their results to refer and compare them later.
Do you think it is better to keep "valueMissing==false" ?

>> LayoutTests/fast/forms/ValidityState-valueMissing-003-expected.txt:15
>> +FAIL valueMissingFor("select-none-selected-size2-multiple") should be false. Was true.
> 
> The result contains FAIL lines.

Oh, I missed to replace the the test case after changing the source and my understanding of the spec. "true" is correct for the FAIL cases, without any selected options.

>> LayoutTests/fast/forms/checkValidity-002-expected.txt:11
>> +PASS checkValidityFor("select-non-placeholder") is true
> 
> checkValidity-002.html was for cases with checkValidity()==false.  You shouldn't include checkValidity()==true case if you follow it.

As ValidityState-valueMissing-001.html, this test collects cases for checkValidity().

>> LayoutTests/fast/forms/resources/select-live-pseudo-selectors.js:76
>> +// --------------------------------
> 
> We need more test cases for this.
> You added some setNeedsValidityCheck() calls.  The test should cover all of code paths with them.

Ok, adding the cases.

>> WebCore/html/HTMLSelectElement.cpp:119
>> +    int index = selectedIndex();
> 
> The variable name should be "firstSelectionIndex" and the comment should be removed.

Exactly, done it. Thanks.

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