|Summary:||Change the timing of form state restore|
|Product:||WebKit||Reporter:||Kent Tamura <tkent>|
|Component:||Forms||Assignee:||Kent Tamura <tkent>|
|Severity:||Normal||CC:||beidson, jonlee, mifenton, morrita, sullivan, webkit-bug-importer, webkit.review.bot|
|Version:||528+ (Nightly build)|
|Bug Depends on:|
|Bug Blocks:||23346, 91209|
Description Kent Tamura 2012-06-26 02:31:58 PDT
See https://bugs.webkit.org/show_bug.cgi?id=23346#c45. At this moment, we restore form control state when we have finished parsing HTML source of the control. I'd like to change it so that restoring just after 'load' event of the document.
Comment 1 Jon Lee 2012-06-26 23:29:34 PDT
Do we have an idea of how other browsers behave?
Comment 3 Brady Eidson 2012-06-27 08:42:28 PDT
(In reply to comment #0) > I'd like to change it so that restoring just after 'load' event of the document. What do relevant specs say? What do all other browsers do? I understand it would be quite convenient to make this change, but convenience doesn't always get to drive our decisions!
Comment 4 Kent Tamura 2012-06-27 17:58:41 PDT
I don't think there are relevant spec. Behavior of other browsers: Firefox: A value was restored before 'load' event of the document, and the restoring dispatched 'input' event for the control. (I tested with an <input>) IE: A value was restored before 'load' event of the document, and the restoring didn't dispatch 'input' or 'change' event. Opera: It seems Opera doesn't restore form values. Hmm, so restoring after 'load' might be risky. Let me reconsider.
Comment 5 Kent Tamura 2012-07-12 22:59:52 PDT
I'd like to change the timing so that: - Restoring in finishParsingChildren of owner form for form controls with an owner - Restoring in finishParsingChildren of form control itself for form control without any owner (no change) Because - Restoring after 'load' event is not compatibile with other browsers - 'load' event may be too late. It can be dispatched several seconds later after starting the document loading - In order to fix a case of http://code.google.com/p/chromium/issues/detail?id=34351#c10, we need to take a look at the <form> content.
Comment 7 Hajime Morrita 2012-07-13 02:30:50 PDT
Comment on attachment 152178 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=152178&action=review I hope that the restoring policy be clarified or documented somehow. For example, Can we add kind of "dirty bit" for each state to prevent accidental duplication? > Source/WebCore/ChangeLog:8 > + For a preparation to fix a form identification problem, restore controls Could you point the bug id here? > Source/WebCore/ChangeLog:35 > + Add isFormControlElementWithState() for FormController::restoreControlStateIn(). Nit: Added > Source/WebCore/ChangeLog:38 > + Some code is moved to FormController:restoreControlStateFor(). Nit: s/is/was/ > Source/WebCore/html/FormController.cpp:34 > + // Assume contorl with form attribute have no owners because we restores s/contorl/controls/ s/restores/restore/
Comment 8 Kent Tamura 2012-07-13 02:52:34 PDT
Comment on attachment 152178 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=152178&action=review >> Source/WebCore/ChangeLog:8 >> + For a preparation to fix a form identification problem, restore controls > > Could you point the bug id here? Sure. I have fined Bug 91209, and will add it to ChangeLog.
Comment 9 Kent Tamura 2012-07-13 02:54:20 PDT
(In reply to comment #7) > I hope that the restoring policy be clarified or documented somehow. I'll make a wiki page later.
Comment 10 Kent Tamura 2012-07-13 02:57:46 PDT
Created attachment 152199 [details] Patch for landing
Comment 11 WebKit Review Bot 2012-07-13 04:00:18 PDT
Comment on attachment 152199 [details] Patch for landing Clearing flags on attachment: 152199 Committed r122559: <http://trac.webkit.org/changeset/122559>
Comment 12 WebKit Review Bot 2012-07-13 04:00:24 PDT
All reviewed patches have been landed. Closing bug.