Bug 31539 - Form Reset : Press Back Button and Reset , TextareaElemnt is not set to Default Value
: Form Reset : Press Back Button and Reset , TextareaElemnt is not set to Defau...
Status: RESOLVED FIXED
: WebKit
Forms
: 528+ (Nightly build)
: PC Mac OS X 10.5
: P1 Normal
Assigned To:
: http://spe.mobilephone.net/wit/witroo...
:
:
:
  Show dependency treegraph
 
Reported: 2009-11-16 02:45 PST by
Modified: 2010-01-12 03:48 PST (History)


Attachments
Test part 1 (2.57 KB, text/html)
2009-12-01 08:40 PST, Ben Murdoch
no flags Details
Test part 2 - place in resources directory under part 1 (409 bytes, text/html)
2009-12-01 08:41 PST, Ben Murdoch
no flags Details
Proposed patch (1.35 KB, patch)
2009-12-07 11:22 PST, Ben Murdoch
no flags Review Patch | Details | Formatted Diff | Diff
Fix with manual test (3.82 KB, patch)
2010-01-11 09:58 PST, Ben Murdoch
no flags Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2009-11-16 02:45:29 PST
I visited  the formreset
link(http://spe.mobilephone.net/wit/xhtmlv2/formreset.xhtml ).

All Radio button , checkbox , TextArea has some defalut value .
After modifying it and pressing Submit , the data gets submitted , If we press back and Press a Reset Button . The textarea feild is not getting reset to default valu e,

I observer the same behavior in other Browser(Mozilla) , its resets the
same .


Regards ,
 Amol
------- Comment #1 From 2009-11-16 17:38:27 PST -------
I cannot reproduce this with Safari 4.0.3 on Mac OS X. What browser are you seeing this with?
------- Comment #2 From 2009-11-16 21:22:33 PST -------
Hi Alexey,
Thanks for the reply .
I can see this issue in Google Chrome(3.0.195.33) which i have installed .

I have ported the Webkit Browser code and the same issue i can see in Google android phone .


Thanks and Regards ,
 Amol
------- Comment #3 From 2009-11-27 06:45:16 PST -------
I had a look at this on Android, and the problem seems to be when TextAreaElement::restoreFormControlElementState is called when we navigate back to the page. This resets the *default* value of the element to the state of the form before the submit (using TextAreaElement::setDefaultValue), so when we click reset, the default value to reset to matches the current state rather than the default value stipulated by the HTML.

Setting breakpoints in Safari, it seems restoreFormControlElementState isn't getting called because it's being pulled out of the cache and short circuiting some code, whereas on Android we are triggering a new load. If after submitting the form you clear Safari's cache and then go back, the bug surfaces and you cannot reset the text area to it's default value of 123.

I think the fix therefore is to call setValue rather than setDefaultValue in restoreFormControlElementState. I can send a patch for this.

Thanks, Ben
------- Comment #4 From 2009-11-30 03:04:57 PST -------
I have a patch ready to fix this bug, but before I upload it I've been trying to write a layout test for it. It's tricky because to expose the bug on Safari, you need to clear the cache between submitting the form and hitting back. Is there a way to turn off or clear the cache through the layout test controller?

Thanks, Ben
------- Comment #5 From 2009-11-30 08:35:28 PST -------
Maybe setCacheModel(0) will work?
------- Comment #6 From 2009-12-01 08:29:17 PST -------
I have written  a reduced test case that exposes the bug on Safari which is perfect for the layout test. However, I'm having trouble getting the test to execute automatically. If you open the test page and manually click the 'submit' button on the form in the iframe, then the test will expose the bug. When I try to automate this by programattically clicking the button, the test does not run correctly. I think I've tracked the problem down to the fact that it seems that for events from script that trigger navigation, no history items are created and so the call to history.back() in the form handler fails.

I will attach the test as I have it right now; does any one have any advice as to how I could fully automate it? I've tried using the eventSender object in DRT, calling the submit button's click() method, instantiating an Event object and dispatching it to the submit button, altering location.hash to try and create history entries, calling the submit() function on the form itself, but nothing seems to work...

The fix for the bug is a very simple one line change in HTMLTextAreaElement.cpp.

Thanks, Ben
------- Comment #7 From 2009-12-01 08:40:39 PST -------
Created an attachment (id=44083) [details]
Test part 1

First part of the test
------- Comment #8 From 2009-12-01 08:41:56 PST -------
Created an attachment (id=44084) [details]
Test part 2 - place in resources directory under part 1
------- Comment #9 From 2009-12-07 11:22:12 PST -------
I haven't had any luck getting the test to automate - I wonder if we could consider adding the fix without a test? I have attached a patch...
------- Comment #10 From 2009-12-07 11:22:57 PST -------
Created an attachment (id=44422) [details]
Proposed patch
------- Comment #11 From 2009-12-08 11:46:45 PST -------
(From update of attachment 44422 [details])
>  void HTMLTextAreaElement::restoreFormControlState(const String& state)
>  {
> -    setDefaultValue(state);
> +    setValue(state);
>  }

Sure, we want to set the value. But don't we want to set the default value too?
------- Comment #12 From 2009-12-08 11:47:35 PST -------
style-queue ran check-webkit-style on attachment 44422 [details] without any errors.
------- Comment #13 From 2009-12-08 11:54:14 PST -------
(In reply to comment #11)
> 
> Sure, we want to set the value. But don't we want to set the default value too?

I don't think so -- not to the value that was stored as the state of the text area, anyway?
------- Comment #14 From 2009-12-29 05:45:50 PST -------
(In reply to comment #11)
> (From update of attachment 44422 [details] [details])
> >  void HTMLTextAreaElement::restoreFormControlState(const String& state)
> >  {
> > -    setDefaultValue(state);
> > +    setValue(state);
> >  }
> 
> Sure, we want to set the value. But don't we want to set the default value too?

I don't think that's right - when saving form control state, what we saved originally was the value, not the default value. We could save the default value as well (in case it was changed e.g. by altering the <textarea>'s DOM children) but in general our form control state saving code doesn't try to do that type of thing.
------- Comment #15 From 2010-01-08 23:48:04 PST -------
How about adding the test as a manual test?
------- Comment #16 From 2010-01-11 04:50:57 PST -------
(In reply to comment #15)
> How about adding the test as a manual test?

That would do it I should think, thanks for the tip! I've never written or used manual tests in WebKit -- is it as one would expect, a layout test that is designed to be run by hand? A quick look at current tests inside WebCore/manual-tests would suggest so and that the body of the test page should include instructions for running the test and the expected results. Are there any other guidelines or tips for writing a manual test? Are these tests run with any regularity?

Thanks, Ben
------- Comment #17 From 2010-01-11 09:58:22 PST -------
Created an attachment (id=46286) [details]
Fix with manual test

Here's the same patch with a manual test.
------- Comment #18 From 2010-01-12 02:29:15 PST -------
(From update of attachment 46286 [details])
cq+ for landing.
------- Comment #19 From 2010-01-12 03:48:12 PST -------
(From update of attachment 46286 [details])
Clearing flags on attachment: 46286

Committed r53132: <http://trac.webkit.org/changeset/53132>
------- Comment #20 From 2010-01-12 03:48:20 PST -------
All reviewed patches have been landed.  Closing bug.