Bug 57636 - Setting defaultValue on a textarea with a modified value still clobbers the value
Summary: Setting defaultValue on a textarea with a modified value still clobbers the v...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Forms (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-04-01 09:02 PDT by Boris Zbarsky
Modified: 2011-04-05 07:29 PDT (History)
9 users (show)

See Also:


Attachments
Testcase (187 bytes, text/html)
2011-04-01 09:14 PDT, Boris Zbarsky
no flags Details
Patch (6.79 KB, patch)
2011-04-04 00:17 PDT, Kent Tamura
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Boris Zbarsky 2011-04-01 09:02:34 PDT
BUILD: Safari 5, Chrome 12 dev

STEPS TO REPRODUCE: Load attached testcase.

EXPECTED RESULT: Textarea says "PASS"

ACTUAL RESULT: Textarea says "FAIL"
Comment 1 Boris Zbarsky 2011-04-01 09:14:39 PDT
Created attachment 87867 [details]
Testcase
Comment 2 Alexey Proskuryakov 2011-04-01 10:34:46 PDT
Wow.
Comment 3 Boris Zbarsky 2011-04-01 11:01:15 PDT
Oh, the original context I ran into this in the page was setting .innerHTML, not .defaultValue.  But I assume they end up in the same place in the end: modifying the DOM under the textarea.  ;)  The innerHTML case shows the same problem, of course.
Comment 4 Kent Tamura 2011-04-01 21:29:15 PDT
IE 9: PASS
Firefox 4: PASS
Opera 11:  FAIL
Comment 5 Boris Zbarsky 2011-04-01 21:39:41 PDT
Quite interesting.  In Opera 11, scripted value changes (as in the attached testcase) on <textarea> allow defaultValue to override value, but if I edit the text in the textarea by typing (as in the original testcase I based mine on) then defaultValue changes don't affect value.  I wasn't expecting Opera to have a difference in behavior there.  None of the other three rendering engines involved seem to: WebKit shows "FAIL" in both scenarios, while Trident and Gecko show "PASS".

Note also that WebKit's behavior here is inconsistent for <textarea> and <input type="text">; the latter behaves as it does in Trident and Gecko.
Comment 6 Kent Tamura 2011-04-01 21:50:21 PDT
Yeah, definitely we should fix this.
Comment 7 Kent Tamura 2011-04-04 00:17:42 PDT
Created attachment 88037 [details]
Patch
Comment 8 Dimitri Glazkov (Google) 2011-04-04 07:57:34 PDT
Comment on attachment 88037 [details]
Patch

ok.
Comment 9 Darin Adler 2011-04-04 08:33:11 PDT
Comment on attachment 88037 [details]
Patch

I think we can eliminate setNonDirtyValue and setValueCommon. Instead, callers can just call setValue and then set m_isDirty to false.
Comment 10 Kent Tamura 2011-04-04 19:17:55 PDT
(In reply to comment #9)
> (From update of attachment 88037 [details])
> I think we can eliminate setNonDirtyValue and setValueCommon. Instead, callers can just call setValue and then set m_isDirty to false.

setValue() needs to call setNeedsValidityCheck(), which must be called after m_isDirty update.  So, the current code is efficient.
Comment 11 Kent Tamura 2011-04-05 00:27:50 PDT
Comment on attachment 88037 [details]
Patch

Clearing flags on attachment: 88037

Committed r82908: <http://trac.webkit.org/changeset/82908>
Comment 12 Kent Tamura 2011-04-05 00:27:58 PDT
All reviewed patches have been landed.  Closing bug.
Comment 13 WebKit Review Bot 2011-04-05 01:34:12 PDT
http://trac.webkit.org/changeset/82908 might have broken GTK Linux 32-bit Release
Comment 14 Philippe Normand 2011-04-05 02:07:23 PDT
(In reply to comment #13)
> http://trac.webkit.org/changeset/82908 might have broken GTK Linux 32-bit Release

fast/forms/ValidityState-tooLong-textarea.html	now fails on GTK:

--- /home/slave/webkitgtk/gtk-linux-64-debug/build/layout-test-results/fast/forms/ValidityState-tooLong-textarea-expected.txt	2011-04-05 01:05:58.793116363 -0700
+++ /home/slave/webkitgtk/gtk-linux-64-debug/build/layout-test-results/fast/forms/ValidityState-tooLong-textarea-actual.txt	2011-04-05 01:05:58.793116363 -0700
@@ -31,8 +31,8 @@
 PASS textarea.value is "abc"
 PASS textarea.validity.tooLong is false
 PASS textarea.validity.tooLong is true
-PASS textarea.value is "abcdef"
-PASS textarea.validity.tooLong is false
+FAIL textarea.value should be abcdef. Was def.
+FAIL textarea.validity.tooLong should be false. Was true.
 PASS successfullyParsed is true
 
 TEST COMPLETE
Comment 15 Kent Tamura 2011-04-05 02:11:53 PDT
(In reply to comment #14)
> (In reply to comment #13)
> > http://trac.webkit.org/changeset/82908 might have broken GTK Linux 32-bit Release
> 
> fast/forms/ValidityState-tooLong-textarea.html    now fails on GTK:

Thank you for reporting.  I fixed it by r82913.
Comment 16 Simon Pieters (:zcorpan) 2011-04-05 03:28:38 PDT
Relevant parts of the spec

[[ 
  The textarea element represents a multiline plain text edit control for the element's raw value. 

  A textarea element has a dirty value flag, which must be initially set to false, and must be set to true whenever the user interacts with the control in a way that changes the raw value. 

  When the textarea element's textContent IDL attribute changes value, if the element's dirty value flag is false, then the element's raw value must be set to the value of the element's textContent IDL attribute. 

  The defaultValue IDL attribute must act like the element's textContent IDL attribute. 
  ]] 
http://www.whatwg.org/specs/web-apps/current-work/complete/the-button-element.html#the-textarea-element 

AFAICT the old behavior matches the spec. (Setting .value doesn't set the dirty value flag.)
Comment 17 Boris Zbarsky 2011-04-05 07:29:28 PDT
I believe that's a bug in the spec, since for <input> setting .value _does_ change the dirty flag in the spec.  Filed http://www.w3.org/Bugs/Public/show_bug.cgi?id=12423