Bug 89539

Summary: Fix a form restore test broken by HTML5 parser
Product: WebKit Reporter: Kent Tamura <tkent>
Component: Tools / TestsAssignee: Kent Tamura <tkent>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, eric, haraken, morrita, rafaelw, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 88685, 89409    
Attachments:
Description Flags
Patch
none
Patch 2 haraken: review+

Kent Tamura
Reported 2012-06-19 18:37:32 PDT
http://trac.webkit.org/changeset/66482 made fast/forms/restore-to-non-edited-controls.html nonsense. We fix the test in this bug.
Attachments
Patch (4.62 KB, patch)
2012-06-19 18:48 PDT, Kent Tamura 		no flags
DetailsFormatted DiffDiff
Patch 2 (4.62 KB, patch)
2012-06-19 19:24 PDT, Kent Tamura 		haraken: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Kent Tamura
Comment 1 2012-06-19 18:48:34 PDT
Created attachment 148481 [details] Patch
Kentaro Hara
Comment 2 2012-06-19 19:22:02 PDT
Comment on attachment 148481 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=148481&action=review > LayoutTests/ChangeLog:13 > + After HTML5 parser, we don't attach renderes during innerHTML Nit: renderes => renderers > LayoutTests/ChangeLog:15 > + built a form and the test haven't work since introducing HTML5 parser. Just a confirmation: What happens if innerHTML is used and <!DOCTYPE html> is omitted? I am asking this because I am not sure whether we should hide the difference by using document.write() (just like your patch) or we should test innerHTML for both <!DOCTYPE html> case and no-<!DOCTYPE html> case.
Kent Tamura
Comment 3 2012-06-19 19:23:57 PDT
Comment on attachment 148481 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=148481&action=review >> LayoutTests/ChangeLog:15 >> + built a form and the test haven't work since introducing HTML5 parser. > > Just a confirmation: What happens if innerHTML is used and <!DOCTYPE html> is omitted? > > I am asking this because I am not sure whether we should hide the difference by using document.write() (just like your patch) or we should test innerHTML for both <!DOCTYPE html> case and no-<!DOCTYPE html> case. DOCTYPE doesn't matter. We always use HTML5 parser.
Kent Tamura
Comment 4 2012-06-19 19:24:57 PDT
Created attachment 148487 [details] Patch 2 typo
Kentaro Hara
Comment 5 2012-06-19 19:28:27 PDT
Comment on attachment 148487 [details] Patch 2 Thanks for the clarification.
Eric Seidel (no email)
Comment 6 2012-06-19 19:57:26 PDT
I'm confused. If we changed innerHTML behavior, was this a regression? Does our new innerHTML/form behavior match other browsers? It's good that we fixed the test, but there may be an underlying bug here that we're pasting over?
Kent Tamura
Comment 7 2012-06-19 20:23:56 PDT
(In reply to comment #6) > I'm confused. If we changed innerHTML behavior, was this a regression? Does our new innerHTML/form behavior match other browsers? > > It's good that we fixed the test, but there may be an underlying bug here that we're pasting over? Yeah, HTML5 parser made a regression. We were able to restore form values for dynamically generated form with innerHTML, but now we can't. However I haven't seen bug reports for this regression. I guess this is not so important to users. I think there are no ways to observe whether a node is attached in Node::finishParsingChildren() in innerHTML case on other browsers, and this is not a compatibility issue. I have a plan to change the form restore timing. The regression will be resolved then.
Eric Seidel (no email)
Comment 8 2012-06-19 20:31:46 PDT
(In reply to comment #7) > (In reply to comment #6) > I have a plan to change the form restore timing. The regression will be resolved then. Do we have a bug to track that change. Seems useful to relate here. Thanks!
Kent Tamura
Comment 9 2012-06-19 20:35:25 PDT
(In reply to comment #8) > (In reply to comment #7) > > (In reply to comment #6) > > I have a plan to change the form restore timing. The regression will be resolved then. > > Do we have a bug to track that change. Seems useful to relate here. Thanks! I explained the plan in https://bugs.webkit.org/show_bug.cgi?id=23346#c45 . I'll add a new blocker bug for the timing change to Bug 23346.
Kent Tamura
Comment 10 2012-06-19 21:26:28 PDT
Committed r120796: <http://trac.webkit.org/changeset/120796>
Note You need to log in before you can comment on or make changes to this bug.