Bug 89962 – Change the timing of form state restore

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

[Kent Tamura]

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.

[Jon Lee]

Do we have an idea of how other browsers behave?

[Radar WebKit Bug Importer]

<rdar://problem/11757271>

[Brady Eidson]

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

[Kent Tamura]

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.

[Kent Tamura]

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.

[Kent Tamura]

Created attachment 152178 [details]
Patch

[Hajime Morrita]

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/

[Kent Tamura]

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.

[Kent Tamura]

(In reply to comment #7)
> I hope that the restoring policy be clarified or documented somehow.

I'll make a wiki page later.

[Kent Tamura]

Created attachment 152199 [details]
Patch for landing

[WebKit Review Bot]

Comment on attachment 152199 [details]
Patch for landing

Clearing flags on attachment: 152199

Committed r122559: <http://trac.webkit.org/changeset/122559>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.