Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Textarea defaultValue handling doesn't really match UAs (who don't match each other) when there are child elements #2752

Closed
bzbarsky opened this issue Jun 12, 2017 · 14 comments · Fixed by #2766
Labels
interop Implementations are not interoperable with each other topic: forms

Comments

@bzbarsky
Copy link
Contributor

Consider this testcase:

<!DOCTYPE html>
<pre><script>
    var t = document.createElement("textarea");
    t.appendChild(document.createElement("span")).appendChild(document.createTextNode("TEXT"));
    document.body.appendChild(t);
    document.writeln("textContent: " + t.textContent);
    document.writeln("defaultValue: " + t.defaultValue);
    document.writeln("value: " + t.value);
  </script>
</pre>

Per spec, https://html.spec.whatwg.org/multipage/form-elements.html#dom-textarea-defaultvalue is supposed to return the value of the textContent IDL attr, and per https://html.spec.whatwg.org/multipage/form-elements.html#the-textarea-element:textcontent the raw value is supposed to match the textContent if the dirty flag is false. So per spec as currently written, all three of textContent, defaultValue, value should be "TEXT", and I would think that "TEXT" should appear in the textarea.

Actual observed behavior is:

  • Gecko: textContent is "TEXT", defaultValue/value are "", textarea is empty.
  • WebKit: textContent/defaultValue are "TEXT", value is "", textarea is empty.
  • Blink: textContent is "TEXT", defaultValue/value are "", textarea is empty.
  • Edge: textContent/value are "TEXT", defaultValue is "", textarea shows "TEXT"

Live testcase: https://jsfiddle.net/t9dz1zyj/1/

It looks like Edge is generally pretty buggy on this defaultValue business; the testcase at https://jsfiddle.net/t9dz1zyj/2/ shows "TEXT" for all three properties in Gecko/Blink/WebKit, and should per current spec, but still shows empty defaultValue in Edge...

Anyway, it looks like Edge is trying to do what the spec says, modulo its defaultValue bug. I know for a fact (code inspection) Gecko only considers direct child textnodes for its default value for textarea. Looks like Blink does the same. I'm really not sure what WebKit is doing.

//cc @domenic @travisleithead

@bzbarsky
Copy link
Contributor Author

the testcase at https://jsfiddle.net/t9dz1zyj/2/

That's

<!DOCTYPE html>
<pre><script>
    var t = document.createElement("textarea");
    t.textContent = "TEXT";    
    document.body.appendChild(t);
    document.writeln("textContent: " + t.textContent);
    document.writeln("defaultValue: " + t.defaultValue);
    document.writeln("value: " + t.value);
  </script>
</pre>

@domenic domenic added interop Implementations are not interoperable with each other topic: forms labels Jun 12, 2017
@domenic
Copy link
Member

domenic commented Jun 12, 2017

Combined with #2750 and the related selection bugs, I'm becoming convinced that everything is pretty broken around the textarea spec.

Do you have a suggestion for a reasonable model we could try to converge everyone on, ideally solving both this issue and #2750? And maybe #2424 as well (cf. #2424 (comment))

@bzbarsky
Copy link
Contributor Author

Well, the good news is that Gecko and Blink seem to have interop for purposes of this issue.

The conceptual model in Gecko is as follows:

  • There is an internal slot that stores the default value.
  • There is an internal algorithm to update the value of that slot; this algorithm (let's call it UpdateDefaultValue) sets the slot to the concatenation of the immediate child Text nodes of the textarea. It also updates the raw value if the slot value is changing and the "dirty value" flag is not set.
  • The defaultValue setter sets .textContent.
  • Mutations that would potentially affect .textContent trigger UpdateDefaultValue via an internal hook mechanism, not document observers. This internal hook mechanism doesn't have an equivalent in the DOM spec right now, but I suspect exists in most or all implementations.

There are some complications in terms of what happens if you do these things before the parser is actually done parsing the textarea, though I'm not 100% sure whether that's even a situation you can get into in practice (do we ever insert the textarea into the DOM before we finish parsing its content in a way such that we can end up waiting on network for a bit there and run user script?).

The actual implementation of the Gecko behavior is as follows:

So in practice we're not storing the "default value" in any form other than the actual DOM subtree under the textarea.

@domenic
Copy link
Member

domenic commented Jun 12, 2017

OK, great. It sounds like the majority of spec work here will be specifying the "something that would affect textContent". I think my plan would be:

  • Put a placeholder hook into the DOM spec, e.g. "text content changed steps", which is defined lamely (e.g. "whenever something happens that would affect the value returned by textContent") with a TODO to formalize it later.
  • Implement your above model in the spec (with either an explicit default value slot, or not)
  • Also use that model to build a solution for the selection issue, since IIRC we landed on that being the crux of specifying that as well
  • Write lots of tests, and in the process, maybe figure out how to spec the lamely-specced DOM hook.

@tkent-google
Copy link
Contributor

I think Gecko and Blink behavior, concatenating Text children, is reasonable. If TEXTAREA needs to react to any textContent change, DOM mutation operations need to be notified to all of TEXTAREA ancestors, and it would slow down DOM mutation without TEXTAREA too.

WebKit's behavior is almost same as Gecko and Blink internally. It just returns textContent for defaultValue IDL attribute getter.

@domenic
Copy link
Member

domenic commented Jun 14, 2017

Hmm, I skimmed over the difference between textContent and contenation of immediately child text nodes. The spec currently leans hard on the connection to textContent, e.g.

The reset algorithm for textarea elements is to set the dirty value flag back to false, and set the raw value of element to the value of the element's textContent IDL attribute.

It sounds like we can match Firefox/Chrome by converging away from textContent to the concatenation of child text nodes.


Here is my proposed spec text, replacing a few existing paragraphs (Ctrl+F for "textContent" in the textarea section of the spec):

A textarea element has the following child text changed steps:

  1. If the textarea element's dirty value flag is true, then return.
  2. Set the textarea element's raw value to its child text.

[child text changed and child text are defined as you would expect, in DOM I guess.]

The reset algorithm for textarea elements is to set the dirty value flag to false, and set the raw value of element to its child text.

If the textarea element has a maximum allowed value length, then the element's children must be such that the JavaScript string length of the value of the element's textContent IDL attribute with the textarea line break normalization transformation applied is equal to or less than the element's maximum allowed value length. [no change; this is a conformance requirement on authors who should not be inserting child elements into textarea elements]

The defaultValue IDL attribute must return the element's child text.


Does this sound good to you, @bzbarsky and @tkent-google? @cdumez might enjoy this as well, as even though Safari doesn't match the proposal/Firefox/Blink, it also doesn't match the current spec.

@bzbarsky
Copy link
Contributor Author

That sounds reasonable to me. We should really check with Microsoft, since they're the closest to what the current spec says, though still not matching it.

@zcorpan
Copy link
Member

zcorpan commented Jun 15, 2017

https://html.spec.whatwg.org/#child-text-content is a concept the spec has already for a bunch of things, FWIW.

@zcorpan
Copy link
Member

zcorpan commented Jun 15, 2017

Is there a web compat need to flip the dirty flag if textContent changes but "child text content" doesn't? Is that hook used for anything other than textarea? I'm wondering if it's possible (and if there's interest) to make this simpler and basically only consider the children of textarea.

@zcorpan
Copy link
Member

zcorpan commented Jun 15, 2017

cc @travisleithead

@bzbarsky
Copy link
Contributor Author

Is there a web compat need to flip the dirty flag if textContent changes but "child text content" doesn't?

I don't follow. The dirty flag is never changed by changes to either "textContent" or "child text content". What those changes do is, if the dirty flag is not set, set the raw value to the "child text content" (or "textContent", in current spec). They do NOT affect the dirty flag.

@zcorpan
Copy link
Member

zcorpan commented Jun 15, 2017

Ok, sorry, I was clearly confused.

So if the dirty flag is not set, can we not set the raw value when non-child descendants change?

@bzbarsky
Copy link
Contributor Author

bzbarsky commented Jun 15, 2017

That is the proposal, yes.

Though as long as we set it to "child text content", it doesn't matter whether we do it when only kids change or when non-child descendants change, right? The observables are the same.

@domenic
Copy link
Member

domenic commented Jun 15, 2017

I seem to have lost a step in my above set, which does nothing if the dirty value flag is true. I edited to add that. That might be causing some of the confusion.

domenic added a commit to whatwg/dom that referenced this issue Jun 15, 2017
Part of fixing whatwg/html#2752. The "child text content" concept was previously defined in HTML, but is being moved here.
domenic added a commit to whatwg/dom that referenced this issue Jun 15, 2017
Part of fixing whatwg/html#2752. The "child text content" concept was previously defined in HTML, but is being moved here.
domenic added a commit to whatwg/dom that referenced this issue Jun 15, 2017
Part of fixing whatwg/html#2752. The "child text content" concept was previously defined in HTML, but is being moved here.
domenic added a commit that referenced this issue Jun 15, 2017
This fixes #2752, where it was revealed that most browsers do not care
about the textContent of a textarea, but instead about its child text
content. That is, if you use the DOM APIs to insert a child element into
the textarea element, its text does not show up for editing or as part
of the value and defaultValue properties (even though it shows up in
textContent).

This moves to a model where textareas operate entirely on the child text
content. This matches Blink and Gecko, and somewhat matches WebKit.

This also fixes #2750, by using the newly-introduced "child text content
change steps" hook introduced to DOM in
whatwg/dom#466.
domenic added a commit to whatwg/dom that referenced this issue Jun 27, 2017
Part of fixing whatwg/html#2752. The "child text content" concept was previously defined in HTML, but is being moved here.
domenic added a commit that referenced this issue Jun 27, 2017
This fixes #2752, where it was revealed that most browsers do not care
about the textContent of a textarea, but instead about its child text
content. That is, if you use the DOM APIs to insert a child element into
the textarea element, its text does not show up for editing or as part
of the value and defaultValue properties (even though it shows up in
textContent).

This moves to a model where textareas operate entirely on the child text
content. This matches Blink and Gecko, and somewhat matches WebKit.

This also fixes #2750, by using the newly-introduced "child text content
change steps" hook introduced to DOM in
whatwg/dom#466.
alice pushed a commit to alice/html that referenced this issue Jan 8, 2019
This fixes whatwg#2752, where it was revealed that most browsers do not care
about the textContent of a textarea, but instead about its child text
content. That is, if you use the DOM APIs to insert a child element into
the textarea element, its text does not show up for editing or as part
of the value and defaultValue properties (even though it shows up in
textContent).

This moves to a model where textareas operate entirely on the child text
content. This matches Blink and Gecko, and somewhat matches WebKit.

This also fixes whatwg#2750, by using the newly-introduced "child text content
change steps" hook introduced to DOM in
whatwg/dom#466.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
interop Implementations are not interoperable with each other topic: forms
Development

Successfully merging a pull request may close this issue.

4 participants