Cloning a textarea does not clone the textarea's value

• Component: Forms
• Status: RESOLVED FIXED

[Myles C. Maxfield]

Created attachment 276497 [details]
Reproduction

Open the attached .html file. It should alert "a" twice.

[Myles C. Maxfield]

Note that fixing this bug should include making sure checkValidity() works correctly.

[Myles C. Maxfield]

(In reply to comment #1)
> Note that fixing this bug should include making sure checkValidity() works
> correctly.

fast/forms/checkValidity-cloneNode-crash.html should be updated when this occurs.

[Chris Dumez]

Firefox and Safari alert "a" once.
Chrome alerts "a" twice.

[Chris Dumez]

Myles, why do you expect the textarea cloning to clone the value? I am looking at the spec but I cannot find the part that says we should (maybe I missed it or maybe it is a bug in the spec).

There is what I found in the spec:
https://dom.spec.whatwg.org/#concept-node-clone

-> Says we should copy over all the element attributes, but 'value' is not a content attribute of textarea.
-> Says to run any cloning steps defined for the element in other applicable specifications

So I looked at the HTML spec for HTMLTextAreaElement but it does not seem to define any additional cloning steps:
https://html.spec.whatwg.org/multipage/forms.html#htmltextareaelement

The only cloning steps I find on this page are:
"The cloning steps for input elements must propagate the value, dirty value flag, checkedness, and dirty checkedness flag from the node being cloned to the copy."

However, HTMLTextAreaElement is not an input element (HTMLInputElement).

So, unless I am mistaken, Firefox and Safari/WebKit both match the specification.
Assuming I did not miss anything and you think we should clone the value, I suggest filing a bug against the HTML spec.

[Ryosuke Niwa]

As far as I read the spec, this is a bug in Chrome and WebKit/Gecko behavior is what the spec mandates.

It's somewhat surprising that input element's value is propagated during cloning.

[Anne van Kesteren]

Per HTML only <input>, <template>, and <script> have special cloning behavior. Rick, Philip, any idea why Chrome has this behavior for <textarea>?

[Philip Jägenstedt]

To begin with, here's the code:
https://chromium.googlesource.com/chromium/src/+blame/8400b8124886e7c94ce59b2e595ae74cee821f56/third_party/WebKit/Source/core/html/HTMLTextAreaElement.cpp#641

That was added in response to this bug:
https://bugs.chromium.org/p/chromium/issues/detail?id=487352

I haven't dug deeper into how this turned out this way, but will comment on the bug and point here.

[Ryosuke Niwa]

Closing this bug as won't fix since WebKit's current behavior matches the spec and Gecko's behavior.  Given this Chrome not matching the spec appears to be unintentional, I don't think we should change our behavior.

[Myles C. Maxfield]

Rather than closing the bug now, if we will proceed by considering this a supported operation, we should add a test.

[Myles C. Maxfield]

Reopening to attach new patch.

[Myles C. Maxfield]

Created attachment 277322 [details]
Patch

[Ryosuke Niwa]

Comment on attachment 277322 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=277322&action=review

We should probably add a more direct test for cloneNode which checks that the value property doesn't get copied over.

> LayoutTests/ChangeLog:7
> +

Please add a change log description.

[Chris Dumez]

Comment on attachment 277322 [details]
Patch

r=me

[Myles C. Maxfield]

Created attachment 277323 [details]
Patch

[Myles C. Maxfield]

Committed r200069: <http://trac.webkit.org/changeset/200069>

[Daniel Bates]

Comment on attachment 277323 [details]
Patch

Clearing review flag given that this patch landed by comment #15.

[Chris Dumez]

Spec discussion on this:
https://github.com/whatwg/html/issues/1233

[Chris Dumez]

Reopen given that the spec was updated got add cloning steps for Textarea:
https://github.com/whatwg/html/pull/1784

[Chris Dumez]

Created attachment 288984 [details]
Patch

[Build Bot]

Comment on attachment 288984 [details]
Patch

Attachment 288984 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/2081456

New failing tests:
fast/forms/checkValidity-cloneNode-crash.html

[Build Bot]

Created attachment 288992 [details]
Archive of layout-test-results from ews114 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews114  Port: mac-yosemite  Platform: Mac OS X 10.10.5

[Sam Weinig]

Comment on attachment 288984 [details]
Patch

Causing crashes.

[Chris Dumez]

Created attachment 289003 [details]
Patch

[Chris Dumez]

Comment on attachment 289003 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=289003&action=review

> Source/WebCore/html/HTMLTextAreaElement.cpp:598
> +    updateValidity();

I was missing a call to updateValidity() here.

[Daniel Bates]

Comment on attachment 289003 [details]
Patch

Please file a new bug for this patch. This patch represents a change of behavior. We should not re-open this bug, which was fixed 5 months ago (comment #15) to conform to the spec. at the time.

[Chris Dumez]

Comment on attachment 289003 [details]
Patch

No, this bug was opened for this purpose, as per the title. At the time, we decided to hold off on making this change (and file a bug upstream). But now that the spec has been updated, we should do it.

This is also the bug that is tracked upstream (by spec editors).

[Chris Dumez]

Comment on attachment 289003 [details]
Patch

Clearing flags on attachment: 289003

Committed r206026: <http://trac.webkit.org/changeset/206026>

[Chris Dumez]

All reviewed patches have been landed.  Closing bug.