HTMLTextAreaElement.defaultValue is not implemented as per spec

HTMLTextAreaElement.defaultValue is not implemented as per spec

According to the HTML5 specification, HTMLTextAreaElement.defaultValue
should be the same as textContent. The special feature in Blink is to
preserve comment nodes in the textarea. Since it's quite hard to get
them into the textarea (contents are CDATA) it seems like a strange
feature. This patch syncs it with the spec.

Spec: http://dev.w3.org/html5/spec-preview/the-textarea-element.html#dom-textarea-defaultvalue

BUG=392843

[Sunil Ratnu, 2015-07-08 13:34:26]

Patchset #1 (id:1) has been deleted

[Sunil Ratnu, 2015-07-10 07:47:26]

sunil.ratnu@samsung.com changed reviewers:
+ tkent@chromium.org

[Sunil Ratnu, 2015-07-10 07:47:26]

Please review this.

https://codereview.chromium.org/1221233003/diff/40001/Source/core/html/HTMLTe...
File Source/core/html/HTMLTextAreaElement.cpp (right):

https://codereview.chromium.org/1221233003/diff/40001/Source/core/html/HTMLTe...
Source/core/html/HTMLTextAreaElement.cpp:422: if (!m_isDirty)
I tried using ImplementedAs=textContent in the idl as you can see in the
previous patchset but that results into failure of test case
"fast/forms/textarea-selection-preservation.html". The call to setNonDirtyValue
ensures that the textarea selection is preserved. Could you please let me know
if there are any issues with this call being here apart from the call to
setTextContent(). If there are any issues, I will try to come up with some
different approach.

[tkent, 2015-07-10 07:59:40]

https://codereview.chromium.org/1221233003/diff/40001/Source/core/html/HTMLTe...
File Source/core/html/HTMLTextAreaElement.cpp (right):

https://codereview.chromium.org/1221233003/diff/40001/Source/core/html/HTMLTe...
Source/core/html/HTMLTextAreaElement.cpp:422: if (!m_isDirty)
On 2015/07/10 07:47:26, Sunil Ratnu wrote:
> I tried using ImplementedAs=textContent in the idl as you can see in the
> previous patchset but that results into failure of test case
> "fast/forms/textarea-selection-preservation.html". The call to
setNonDirtyValue
> ensures that the textarea selection is preserved. Could you please let me know
> if there are any issues with this call being here apart from the call to
> setTextContent(). If there are any issues, I will try to come up with some
> different approach.

Can you add test cases of ta.textContent = '...' to
textarea-selection-preservation.html, similar to existing ta.defaultValue =
'...'?  Do such test cases succeed?

When setTextContent is called, HTMLTextAreaElement::childrenChanged should be
called, then childrenChanged should call setNonDirtyValue.  I wonder why we
still need setNonDirtyValue here.

[Sunil Ratnu, 2015-07-10 13:19:32]

>
https://codereview.chromium.org/1221233003/diff/40001/Source/core/html/HTMLTe...
> File Source/core/html/HTMLTextAreaElement.cpp (right):
> 
>
https://codereview.chromium.org/1221233003/diff/40001/Source/core/html/HTMLTe...
> Source/core/html/HTMLTextAreaElement.cpp:422: if (!m_isDirty)
> On 2015/07/10 07:47:26, Sunil Ratnu wrote:
> > I tried using ImplementedAs=textContent in the idl as you can see in the
> > previous patchset but that results into failure of test case
> > "fast/forms/textarea-selection-preservation.html". The call to
> setNonDirtyValue
> > ensures that the textarea selection is preserved. Could you please let me
know
> > if there are any issues with this call being here apart from the call to
> > setTextContent(). If there are any issues, I will try to come up with some
> > different approach.
> 
> Can you add test cases of ta.textContent = '...' to
> textarea-selection-preservation.html, similar to existing ta.defaultValue =
> '...'?  Do such test cases succeed?


I tried the above i.e. tested for below:

    debug("- set new textContent");
    ta.textContent= 'abc123456';
    shouldBe('ta.selectionStart', '9');
    shouldBe('ta.selectionEnd', '9');

    debug("- set same textContent");
    ta.setSelectionRange(2, 3);
    ta.textContent= 'abc123456';
    shouldBe('ta.selectionStart', '9');
    shouldBe('ta.selectionEnd', '9');

but the test failed with and without this patchset. It fails while setting the
same textContent value. The selectionStart and selectionEnd values should be 9 &
9 respectively but instead comes out to be 2 & 3 respectively.


> When setTextContent is called, HTMLTextAreaElement::childrenChanged should be
> called, then childrenChanged should call setNonDirtyValue.  I wonder why we
> still need setNonDirtyValue here.

I has also thought that this should have been not necessary but the
textarea-selection-preservation.html test fails without this call.

Reason why we need to call setNonDirtyValue(): While debugging I found out that
in testcase#1 (given below), HTMLTextAreaElement::childrenChanged breakpoint was
hit once.
Also in testcase#2, the breakpoint was hit only once. For the second time when
we are changing the default value to the same old one,
HTMLTextAreaElement::childrenChanged is not called.

This is what happens while varying the test case
testcase#1. If we change default value only once

   debug("- set same defaultValue");
   ta.setSelectionRange(2, 3);
   ta.defaultValue = 'abc123456';
   shouldBe('ta.selectionStart', '9');
   shouldBe('ta.selectionEnd', '9');

it passes even if we do not call setNonDirtyValue.

testcase#2. If we add a few more lines, just like the current content of
textarea-selection-preservation.html

    debug("- set new defaultValue");
    ta.defaultValue = 'abc123456';
    shouldBe('ta.selectionStart', '9');
    shouldBe('ta.selectionEnd', '9');

    debug("- set same defaultValue");
    ta.setSelectionRange(2, 3);
    ta.defaultValue = 'abc123456';
    shouldBe('ta.selectionStart', '9');
    shouldBe('ta.selectionEnd', '9');

this does not pass if we do not call setNonDirtyValue(). It passes if we make
the call to setNonDirtyValue().

[tkent, 2015-07-14 01:15:13]

On 2015/07/10 13:19:32, Sunil Ratnu wrote:
> >
>
https://codereview.chromium.org/1221233003/diff/40001/Source/core/html/HTMLTe...
> > File Source/core/html/HTMLTextAreaElement.cpp (right):
> > 
> >
>
https://codereview.chromium.org/1221233003/diff/40001/Source/core/html/HTMLTe...
> > Source/core/html/HTMLTextAreaElement.cpp:422: if (!m_isDirty)
> > On 2015/07/10 07:47:26, Sunil Ratnu wrote:
> > > I tried using ImplementedAs=textContent in the idl as you can see in the
> > > previous patchset but that results into failure of test case
> > > "fast/forms/textarea-selection-preservation.html". The call to
> > setNonDirtyValue
> > > ensures that the textarea selection is preserved. Could you please let me
> know
> > > if there are any issues with this call being here apart from the call to
> > > setTextContent(). If there are any issues, I will try to come up with some
> > > different approach.
> > 
> > Can you add test cases of ta.textContent = '...' to
> > textarea-selection-preservation.html, similar to existing ta.defaultValue =
> > '...'?  Do such test cases succeed?
> 
> 
> I tried the above i.e. tested for below:
> 
>     debug("- set new textContent");
>     ta.textContent= 'abc123456';
>     shouldBe('ta.selectionStart', '9');
>     shouldBe('ta.selectionEnd', '9');
> 
>     debug("- set same textContent");
>     ta.setSelectionRange(2, 3);
>     ta.textContent= 'abc123456';
>     shouldBe('ta.selectionStart', '9');
>     shouldBe('ta.selectionEnd', '9');
> 
> but the test failed with and without this patchset. It fails while setting the
> same textContent value. The selectionStart and selectionEnd values should be 9
&
> 9 respectively but instead comes out to be 2 & 3 respectively.
> 
> 
> > When setTextContent is called, HTMLTextAreaElement::childrenChanged should
be
> > called, then childrenChanged should call setNonDirtyValue.  I wonder why we
> > still need setNonDirtyValue here.
> 
> I has also thought that this should have been not necessary but the
> textarea-selection-preservation.html test fails without this call.
> 
> Reason why we need to call setNonDirtyValue(): While debugging I found out
that
> in testcase#1 (given below), HTMLTextAreaElement::childrenChanged breakpoint
was
> hit once.

> Also in testcase#2, the breakpoint was hit only once. For the second time when
> we are changing the default value to the same old one,
> HTMLTextAreaElement::childrenChanged is not called.

Oh, I see.

How IE and Firefox work with setting defaultValue/textContent and
selectionStart/selectionEnd?

[Sunil Ratnu, 2015-07-14 06:21:41]

On 2015/07/14 01:15:13, tkent wrote:
> On 2015/07/10 13:19:32, Sunil Ratnu wrote:
> > >
> >
>
https://codereview.chromium.org/1221233003/diff/40001/Source/core/html/HTMLTe...
> > > File Source/core/html/HTMLTextAreaElement.cpp (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/1221233003/diff/40001/Source/core/html/HTMLTe...
> > > Source/core/html/HTMLTextAreaElement.cpp:422: if (!m_isDirty)
> > > On 2015/07/10 07:47:26, Sunil Ratnu wrote:
> > > > I tried using ImplementedAs=textContent in the idl as you can see in the
> > > > previous patchset but that results into failure of test case
> > > > "fast/forms/textarea-selection-preservation.html". The call to
> > > setNonDirtyValue
> > > > ensures that the textarea selection is preserved. Could you please let
me
> > know
> > > > if there are any issues with this call being here apart from the call to
> > > > setTextContent(). If there are any issues, I will try to come up with
some
> > > > different approach.
> > > 
> > > Can you add test cases of ta.textContent = '...' to
> > > textarea-selection-preservation.html, similar to existing ta.defaultValue
=
> > > '...'?  Do such test cases succeed?
> > 
> > 
> > I tried the above i.e. tested for below:
> > 
> >     debug("- set new textContent");
> >     ta.textContent= 'abc123456';
> >     shouldBe('ta.selectionStart', '9');
> >     shouldBe('ta.selectionEnd', '9');
> > 
> >     debug("- set same textContent");
> >     ta.setSelectionRange(2, 3);
> >     ta.textContent= 'abc123456';
> >     shouldBe('ta.selectionStart', '9');
> >     shouldBe('ta.selectionEnd', '9');
> > 
> > but the test failed with and without this patchset. It fails while setting
the
> > same textContent value. The selectionStart and selectionEnd values should be
9
> &
> > 9 respectively but instead comes out to be 2 & 3 respectively.
> > 
> > 
> > > When setTextContent is called, HTMLTextAreaElement::childrenChanged should
> be
> > > called, then childrenChanged should call setNonDirtyValue.  I wonder why
we
> > > still need setNonDirtyValue here.
> > 
> > I has also thought that this should have been not necessary but the
> > textarea-selection-preservation.html test fails without this call.
> > 
> > Reason why we need to call setNonDirtyValue(): While debugging I found out
> that
> > in testcase#1 (given below), HTMLTextAreaElement::childrenChanged breakpoint
> was
> > hit once.
> 
> > Also in testcase#2, the breakpoint was hit only once. For the second time
when
> > we are changing the default value to the same old one,
> > HTMLTextAreaElement::childrenChanged is not called.
> 
> Oh, I see.
> 
> How IE and Firefox work with setting defaultValue/textContent and
> selectionStart/selectionEnd?


This is the testcase which I tested:

    var ta = document.getElementById('ta');
    ta.defaultValue = 'abc123456';
    alert(ta.selectionStart + "," + ta.selectionEnd);

    ta.setSelectionRange(2, 3);
    ta.defaultValue = 'abc123456';
    alert(ta.selectionStart + "," + ta.selectionEnd);

    ta.textContent = "abc123456";
    alert(ta.selectionStart + "," + ta.selectionEnd);

    ta.setSelectionRange(2, 3);
    ta.textContent = 'abc123456';
    alert(ta.selectionStart + "," + ta.selectionEnd);

Following are the values for different browsers:

          alert1   alert2   alert3   alert4
    IE:     0,0      2,3      0,0      0,0
    FF:     0,0      9,9      9,9      9,9
    Chrome: 9,9      9,9      9,9      2,3

So, IE keeps the selection start and end at the start of the text i.e. 0,0 until
and unless we specifically set the selection to some values.
    FF, for the first time sets the value to the start of the text (i.e. 0,0).
After that it is always at the end of the text.
    Chrome, for the very first time, sets the values at the end of the text
(i.e. 9,9) but in case of setting same textContent it does not set the values to
   
    9,9, instead it retains the earlier selection of 2,3.

[tkent, 2015-07-14 07:43:19]

On 2015/07/14 06:21:41, Sunil Ratnu wrote:
> This is the testcase which I tested:
> 
>     var ta = document.getElementById('ta');
>     ta.defaultValue = 'abc123456';
>     alert(ta.selectionStart + "," + ta.selectionEnd);
> 
>     ta.setSelectionRange(2, 3);
>     ta.defaultValue = 'abc123456';
>     alert(ta.selectionStart + "," + ta.selectionEnd);
> 
>     ta.textContent = "abc123456";
>     alert(ta.selectionStart + "," + ta.selectionEnd);
> 
>     ta.setSelectionRange(2, 3);
>     ta.textContent = 'abc123456';
>     alert(ta.selectionStart + "," + ta.selectionEnd);
> 
> Following are the values for different browsers:
> 
>           alert1   alert2   alert3   alert4
>     IE:     0,0      2,3      0,0      0,0
>     FF:     0,0      9,9      9,9      9,9
>     Chrome: 9,9      9,9      9,9      2,3
> 
> So, IE keeps the selection start and end at the start of the text i.e. 0,0
until
> and unless we specifically set the selection to some values.
>     FF, for the first time sets the value to the start of the text (i.e. 0,0).
> After that it is always at the end of the text.
>     Chrome, for the very first time, sets the values at the end of the text
> (i.e. 9,9) but in case of setting same textContent it does not set the values
to
>    
>     9,9, instead it retains the earlier selection of 2,3.

Thank you for the investigation.
According to the specification, updating the default value doesn't move
selection.  So, IMO, 
0,0 2,3 0,0 2,3
is what we want.

However the specification doesn't mention the behavior in a case that updated
default value is shorter than selectionStart/selectionEnd.  We need to file a
specification bug.

[Sunil Ratnu, 2015-07-20 12:28:58]

On 2015/07/14 07:43:19, tkent wrote:
> On 2015/07/14 06:21:41, Sunil Ratnu wrote:
> > This is the testcase which I tested:
> > 
> >     var ta = document.getElementById('ta');
> >     ta.defaultValue = 'abc123456';
> >     alert(ta.selectionStart + "," + ta.selectionEnd);
> > 
> >     ta.setSelectionRange(2, 3);
> >     ta.defaultValue = 'abc123456';
> >     alert(ta.selectionStart + "," + ta.selectionEnd);
> > 
> >     ta.textContent = "abc123456";
> >     alert(ta.selectionStart + "," + ta.selectionEnd);
> > 
> >     ta.setSelectionRange(2, 3);
> >     ta.textContent = 'abc123456';
> >     alert(ta.selectionStart + "," + ta.selectionEnd);
> > 
> > Following are the values for different browsers:
> > 
> >           alert1   alert2   alert3   alert4
> >     IE:     0,0      2,3      0,0      0,0
> >     FF:     0,0      9,9      9,9      9,9
> >     Chrome: 9,9      9,9      9,9      2,3
> > 
> > So, IE keeps the selection start and end at the start of the text i.e. 0,0
> until
> > and unless we specifically set the selection to some values.
> >     FF, for the first time sets the value to the start of the text (i.e.
0,0).
> > After that it is always at the end of the text.
> >     Chrome, for the very first time, sets the values at the end of the text
> > (i.e. 9,9) but in case of setting same textContent it does not set the
values
> to
> >    
> >     9,9, instead it retains the earlier selection of 2,3.
> 
> Thank you for the investigation.
> According to the specification, updating the default value doesn't move
> selection.  So, IMO, 
> 0,0 2,3 0,0 2,3
> is what we want.
> 

Thanks for the response tkent. I will try to have a fix so that we can achieve
the desired behaviour.

> However the specification doesn't mention the behavior in a case that updated
> default value is shorter than selectionStart/selectionEnd.  We need to file a
> specification bug.

I will file a specification bug for this as well.