[Webkit-unassigned] [Bug 29363] Support for <output> element

[bugzilla-daemon at webkit.org]

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





--- Comment #9 from Kenichi Ishibashi <bashi at google.com>  2010-11-01 20:07:53 PST ---
(From update of attachment 72521)
View in context: https://bugs.webkit.org/attachment.cgi?id=72521&action=review

Hi Kent-san,

Thank you for your review. I'll send revised patch following your comments.

>> LayoutTests/fast/dom/HTMLOutputElement/script-tests/dom-settable-token-list.js:40
>> +element.htmlFor.add('x'); shouldBeEqualToString('element.htmlFor.toString()', 'x');
> 
> Insert a line-break after ";"

done.

>> LayoutTests/fast/dom/HTMLOutputElement/script-tests/dom-settable-token-list.js:78
>> +debug('Ensure that we can handle empty form attribute correctly');
> 
> Would you add debug('') before this line, or add "-" at the beginning of the message like other tests for readability of the test result please?

I've added "-" at the beginning of all debug messages.

>> LayoutTests/fast/dom/HTMLOutputElement/script-tests/dom-settable-token-list.js:91
>> +element.htmlFor.add('x')
> 
> What's the expected result of these remove() and add()?

I forgot to add expectations. Added expectations. Sorry for confusing.

>> LayoutTests/fast/dom/HTMLOutputElement/script-tests/htmloutputelement-htmlfor.js:1
>> +description('Tests for the htmlFor attribute of the output element.');
> 
> Many test cases in this file look equivalent to tests in dom-settable-token-list.js.  Are they needed?

Removed.

>> LayoutTests/fast/dom/HTMLOutputElement/script-tests/htmloutputelement-value.js:7
>> +output  = document.createElement('output');
> 
> Redundant spaces before "="

Fixed.

>> WebCore/html/HTMLOutputElement.cpp:83
>> +void HTMLOutputElement::childrenChanged(bool createdByParser, Node* /*beforeChange*/, Node* /*afterChange*/, int /*childCountDelta*/)
> 
> You don't need to keep unused parameter names.

Thank you for letting me know that. Removed unused parameter names.

>> WebCore/html/HTMLOutputElement.idl:35
>> +        readonly attribute boolean willValidate;
> 
> You need a test for willValidate behavior.

Thank you for the advice. I've added a test.

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