Bug 26241 – Form control state shouldn't be restored for hidden inputs.

• Status: RESOLVED
• Resolution: FIXED
• Component: Forms

[Kato Kazuyoshi]

Overview Description:
WebKit save/restore value of a input element from WebCore::HistoryItem. But this behavior seems strange at non-bfcached page.

Steps to Reproduce:

1) Create a simple "counter" app. that returns the following HTML. And #{count} is incremented by each access.

<form method="get" action="/foo">
  #{count}
  <input type="hidden" name="count" value="#{count}"/>
  <input type="submit" />
</form>
<iframe src="http://webkit.org/"></iframe>

2) Access the page and press "Submit" button. New page is "/foo?count=1".

3) Press browser's "Back" button. WebKit requests the page agin because bfcache is disabled by the iframe.

4) The application *increments counter* and returns HTML again.

5) Press "Submit" again.

Expected Results:
New page is should be "/foo?count=2" because WebKit requests the page and the server returns new HTML.

Actual Results:
New page is "/foo?count=1". Because WebCore::FrameLoader::restoreDocumentState and WebCore::HTMLInputElement::restoreState replaced value of input elements by the old (saved) values.

This behavior sometimes useful. When user input long (or complex) value, WebKit should save the value and user can notice when "restore" is wrong. But I think, WebKit don't need to save/restore value of <input type="hidden" .../> because user don't input the value and user can't notice WebKit's "restore".

Index: WebCore/html/HTMLInputElement.cpp
===================================================================
--- WebCore/html/HTMLInputElement.cpp	(revision 44489)
+++ WebCore/html/HTMLInputElement.cpp	(working copy)
@@ -422,7 +422,6 @@
         case BUTTON:
         case EMAIL:
         case FILE:
-        case HIDDEN:
         case IMAGE:
         case ISINDEX:
         case NUMBER:
@@ -440,6 +439,7 @@
             setChecked(state == "on");
             break;
         case PASSWORD:
+        case HIDDEN:
             break;
     }
 }

[Dimitri Glazkov (Google)]

I think it's reasonable for hidden input state not to be restored. Darin, Alexey, WDYT?

[Dimitri Glazkov (Google)]

I can prepare a patch/reduction if you think it's a good idea.

[Darin Adler]

The discussion here makes no mention of other web browsers or HTML5. Could you check to see what the behavior is in IE and Firefox and also whether the HTML5 specification prescribes specific behavior for this case?

[Alexey Proskuryakov]

You may want to have a look at bug 23346 and bug 8613. These seem vague, with extremely unhelpful titles, but hopefully there is some insight anyway. Would be nice to re-title this bug, too.

[Dimitri Glazkov (Google)]

(In reply to comment #3)
> The discussion here makes no mention of other web browsers or HTML5. Could you
> check to see what the behavior is in IE and Firefox and also whether the HTML5
> specification prescribes specific behavior for this case?

Neither Firefox nor IE do this. HTML5 spec doesn't define behavior or restoring/saving form values on navigation. It just mentions that user agents may persist the form controls state: http://www.whatwg.org/specs/web-apps/current-work/multipage/history.html#history.

[Alexey Proskuryakov]

Another possibly related issue: bug 31413.

[Darin Adler]

(In reply to comment #5)
> (In reply to comment #3)
> > The discussion here makes no mention of other web browsers or HTML5. Could you
> > check to see what the behavior is in IE and Firefox and also whether the HTML5
> > specification prescribes specific behavior for this case?
> 
> Neither Firefox nor IE do this.

Neither restore form control state at all? Or neither restore form control state for <input type=hidden>?

[Kent Tamura]

It seems behavior of IE and Firefox is just restoring the whole DOM tree and clearing type=password.

[Darin Adler]

(In reply to comment #8)
> It seems behavior of IE and Firefox is just restoring the whole DOM tree and
> clearing type=password.

Form values aren’t in the DOM tree, so I’m not sure what you mean exactly.

[Kent Tamura]

(In reply to comment #9)
> (In reply to comment #8)
> > It seems behavior of IE and Firefox is just restoring the whole DOM tree and
> > clearing type=password.
> 
> Form values aren’t in the DOM tree, so I’m not sure what you mean exactly.

HTMLInputElement::value is a part of the DOM tree, isn't it?
http://www.w3.org/TR/DOM-Level-2-HTML/html.html#ID-6043025

Anyway, I meant IE/Firefox had a cache for rendered pages.  I guess they don't delete objects representing a page when a user leaves the page and visits another page.  So their `Back' operation don't re-fetch the page, don't parse the HTML, and just show the objects on memory.

[Darin Adler]

(In reply to comment #10)
> Anyway, I meant IE/Firefox had a cache for rendered pages.  I guess they don't
> delete objects representing a page when a user leaves the page and visits
> another page.  So their `Back' operation don't re-fetch the page, don't parse
> the HTML, and just show the objects on memory.

That may be true in some cases, but it's definitely not true in all cases. If that were true, then the browsers would have to keep all pages in your entire back history in memory, and I assure you they do not.

[Kent Tamura]

Created attachment 52400 [details]
Proposed patch

[WebKit Commit Bot]

Comment on attachment 52400 [details]
Proposed patch

Rejecting patch 52400 from commit-queue.

Failed to run "['WebKitTools/Scripts/run-webkit-tests', '--no-launch-safari', '--exit-after-n-failures=1', '--quiet']" exit_code: 1
Running build-dumprendertree
Compiling Java tests
make: Nothing to be done for `default'.
Running tests from /Users/eseidel/Projects/CommitQueue/LayoutTests
Testing 12615 test cases.
fast/forms/button-state-restore.html -> failed

Exiting early after 1 failures. 6615 tests run.
109.28s total testing time

6614 test cases (99%) succeeded
1 test case (<1%) had incorrect layout
2 test cases (<1%) had stderr output

Full output: http://webkit-commit-queue.appspot.com/results/1676001

[Kent Tamura]

(In reply to comment #13)
> (From update of attachment 52400 [details])
> Rejecting patch 52400 from commit-queue.
> 
> Failed to run "['WebKitTools/Scripts/run-webkit-tests', '--no-launch-safari',
> '--exit-after-n-failures=1', '--quiet']" exit_code: 1
> Running build-dumprendertree
> Compiling Java tests
> make: Nothing to be done for `default'.
> Running tests from /Users/eseidel/Projects/CommitQueue/LayoutTests
> Testing 12615 test cases.
> fast/forms/button-state-restore.html -> failed

I forgot to add button-state-restore.html to the patch...
It's a simple change to follow the behavior change.

I manually landed the patch with button-state-restore.html as r57013

[Kent Tamura]

*** Bug 37087 has been marked as a duplicate of this bug. ***

[Kent Tamura]

*** Bug 28380 has been marked as a duplicate of this bug. ***