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

[bugzilla-daemon at webkit.org]

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


Kent Tamura <tkent at chromium.org> changed:

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




--- Comment #7 from Kent Tamura <tkent at chromium.org>  2010-11-01 17:45:04 PST ---
(From update of attachment 72521)
View in context: https://bugs.webkit.org/attachment.cgi?id=72521&action=review

> 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 ";"

> 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?

> LayoutTests/fast/dom/HTMLOutputElement/script-tests/dom-settable-token-list.js:80
> +list = element.htmlFor;

Should prepend "var "

> 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()?

> 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?

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

Redundant spaces before "="

> 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.

> WebCore/html/HTMLOutputElement.cpp:133
> +    setTextContent(value, ec);

Better to have ASSERT(!m_isSetTextContentInProgress)

> WebCore/html/HTMLOutputElement.idl:35
> +        readonly attribute boolean willValidate;

You need a test for willValidate behavior.

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