Hi,
This patch is my first attempt to support HTML5 output element. SInce I'm not familiar with adding elements to WebKit, this patch would do something bad, or at least have problems. So It would be definitely helpful for me if you can take a look at the patch and pointed out the problems. This patch also doesn't contain CSS default property for the element. I'll add it later.
Thanks,

Comment on attachment 70950[details]
A trial patch V0
View in context: https://bugs.webkit.org/attachment.cgi?id=70950&action=review
Thank you for working on this.
The patch is too large to review. I recommend to split this into
- DOMSettableTokenList implementation and JSC binding,
- V8 binding for DOMSettableTokenList,
- "form" attribute support for form control elements, and
- <output> element support.
> WebCore/WebCore.xcodeproj/project.pbxproj:14519
> + 4AD7D6631267D56B0020D6E8 /* HTMLOutputElement.cpp */,
You should run WebKitTools/Scripts/sort-Xcode-project-file when you add files to an Xcode project.

Hi Kent-san,
> The patch is too large to review. I recommend to split this into
> - DOMSettableTokenList implementation and JSC binding,
> - V8 binding for DOMSettableTokenList,
> - "form" attribute support for form control elements, and
> - <output> element support.
Thank you for your advice. I'll split this following your advice.
> You should run WebKitTools/Scripts/sort-Xcode-project-file when you add files to an Xcode project.
I didn't know that. Thank you letting me know that!
Regards,

(In reply to comment #5)
Hi,
I've revised the patch. Although the form attribute support, which blocks this issue, is not addressed yet, I think that adding output element before adding form attribute would be more convenient for writing tests (since there is no elements which have the form attribute for now). I'm planning to add form attribute support after this patch is successfully landed.
Thanks,

> > WebCore/html/HTMLOutputElement.cpp:133
> > + setTextContent(value, ec);
>
> Better to have ASSERT(!m_isSetTextContentInProgress)
Done.
> Also, we need to add a default style for <output> to WebCore/css/html.css.
> Probably display:inline or display:inilne-block is sufficient.
> Firefox seems to use display:inline.
Thank you for notifying that. I forgot it. I've added display:inline for <output>.
In addition, I also forgot to add an attribute for HTMLOutputElement to dom/page/Window.idl and added it in the revised patch.
Thanks,

Comment on attachment 72626[details]
Patch V2
View in context: https://bugs.webkit.org/attachment.cgi?id=72626&action=review> LayoutTests/fast/dom/HTMLOutputElement/htmloutputelement-validity-expected.txt:1
> +CONSOLE MESSAGE: line 25: true
Something wrong.
> WebCore/html/HTMLOutputElement.h:44
> + virtual bool isEnumeratable() const { return true; }
Need a test for this.
However, I couldn't find existing tests for from control enumeration. We might need a new test for all of form control types.
These override functions (isEnumeratable, willValidate, reset, childrenChanged) should be moved to private:.

(In reply to comment #14)
> Should this be inside #if?
Does Apple have any special reasons to do so?
If not, I don't think we need to enclose the code by #if ENABLE(). The implementation doesn't need platform-specific code, and will be completed by this patch except "form" attribute support.

Comment on attachment 72626[details]
Patch V2
View in context: https://bugs.webkit.org/attachment.cgi?id=72626&action=review
Kent-san,
Thank you your quick review. I'll revise the patch but I have a question on tests for form control enumeration. If I understand the spec correctly, the spec <http://www.w3.org/TR/html5/forms.html#category-listed> says that following elements would be listed in elements property of form elements: <button>, <fieldset>, <input>, <keygen>, <object>, <output>, <select>, and <textarea>. But for the current implementation, <fieldset>, <keygen> and <object> are not contained in the elements property. Which behavior should I add in the tests?
>> LayoutTests/fast/dom/HTMLOutputElement/htmloutputelement-validity-expected.txt:1
>> +CONSOLE MESSAGE: line 25: true
>
> Something wrong.
I'm sorry I forgot delete this debug information. I'll remove this.
>> WebCore/ChangeLog:7
>> + Support for &lt;output&gt; element
>
> No need to use entity references :-)
Oh, thank you notifying this. I'll fix this.
>> WebCore/html/HTMLOutputElement.h:44
>> + virtual bool isEnumeratable() const { return true; }
>
> Need a test for this.
> However, I couldn't find existing tests for from control enumeration. We might need a new test for all of form control types.
>
> These override functions (isEnumeratable, willValidate, reset, childrenChanged) should be moved to private:.
I've moved these override functions to private except for willValidate() since it is called by JSHTMLOutputElement class.
I'm going to try to write tests for form control enumeration but I have some questions, as I mentioned above. It is great if you could give some advice on this.

(In reply to comment #16)
> Thank you your quick review. I'll revise the patch but I have a question on tests for form control enumeration. If I understand the spec correctly, the spec <http://www.w3.org/TR/html5/forms.html#category-listed> says that following elements would be listed in elements property of form elements: <button>, <fieldset>, <input>, <keygen>, <object>, <output>, <select>, and <textarea>. But for the current implementation, <fieldset>, <keygen> and <object> are not contained in the elements property. Which behavior should I add in the tests?
ok, we need to discuss what we should do. Let's discuss at new bug. You don't need to add an enumeration test in this patch.

Comment on attachment 72910[details]
Patch V4
View in context: https://bugs.webkit.org/attachment.cgi?id=72910&action=review>> LayoutTests/fast/dom/prototype-inheritance-2-expected.txt:-443
>> -
>
> Why is the last line removed?
I'm sorry for confusing, but I've just run new-run-webkit-tests and copied from -actual.txt to -expected.txt. Is this a bug of the new-run-webkit-tests? Anyway, I'll re-send the patch. Sorry again the inconvenience.

(In reply to comment #25)
> I'm sorry for confusing, but I've just run new-run-webkit-tests and copied from -actual.txt to -expected.txt. Is this a bug of the new-run-webkit-tests? Anyway, I'll re-send the patch. Sorry again the inconvenience.
If you work on Mac, I recommend to use run-webkit-tests (not new-).
% run-webkit-tests --debug/--release --reset fast/dom/prototype-inheritance.html
updates the expectation. new-run-webkit-tests on Mac is not popular yet.

(In reply to comment #26)
> If you work on Mac, I recommend to use run-webkit-tests (not new-).
> % run-webkit-tests --debug/--release --reset fast/dom/prototype-inheritance.html
> updates the expectation. new-run-webkit-tests on Mac is not popular yet.
I see. Thank you for your advice. I'll do in this way next time:-)
I've posted revised patch, which will modify platform-dependent expectation files. Thank you for letting me know that.

The commit-queue encountered the following flaky tests while processing attachment 72937[details]:
http/tests/appcache/manifest-redirect.html
Please file bugs against the tests. These tests were authored by ap@webkit.org. The commit-queue is continuing to process your patch.