Form Reset : Press Back Button and Reset , TextareaElemnt is not set to Default Value

• Component: Forms
• Status: RESOLVED FIXED

[Amol]

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

[Alexey Proskuryakov]

I cannot reproduce this with Safari 4.0.3 on Mac OS X. What browser are you seeing this with?

[Amol]

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

[Ben Murdoch]

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

[Ben Murdoch]

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

[Alexey Proskuryakov]

Maybe setCacheModel(0) will work?

[Ben Murdoch]

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

[Ben Murdoch]

Created attachment 44083 [details]
Test part 1

First part of the test

[Ben Murdoch]

Created attachment 44084 [details]
Test part 2 - place in resources directory under part 1

[Ben Murdoch]

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...

[Ben Murdoch]

Created attachment 44422 [details]
Proposed patch

[Darin Adler]

Comment on attachment 44422 [details]
Proposed patch

>  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?

[WebKit Review Bot]

style-queue ran check-webkit-style on attachment 44422 [details] without any errors.

[Ben Murdoch]

(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?

[Maciej Stachowiak]

(In reply to comment #11)
> (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?

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.

[Adele Peterson]

How about adding the test as a manual test?

[Ben Murdoch]

(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

[Ben Murdoch]

Created attachment 46286 [details]
Fix with manual test

Here's the same patch with a manual test.

[Ben Murdoch]

Comment on attachment 46286 [details]
Fix with manual test

cq+ for landing.

[WebKit Commit Bot]

Comment on attachment 46286 [details]
Fix with manual test

Clearing flags on attachment: 46286

Committed r53132: <http://trac.webkit.org/changeset/53132>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.