Bug 44781 – HTMLTreeBuilder needs to call HTMLFormElement::setDemoted

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[Anssi Kolehmainen]

Created attachment 65741 [details]
Incorrect rendering with chrome

Verkkokauppa.com website is rendered incorrectly in latest builds of chrome and Webkit. The search bar ("pikahaku") is moved to right outside the page area. Attached screenshots should show the issue clearly.

Failing browsers: 
Linux, Google Chrome 7.0.503.1 (Official Build 57041) dev, WebKit 534.6
OS X, Webkit nightly, 5.0.1 (6533.17.8, r65398)

Working browsers:
Linux, Firefox 3.0.9
Linux, Konqueror 4.4.4
(Webkit and Chrome probably a month or two ago)

[Anssi Kolehmainen]

Created attachment 65742 [details]
Correct rendering with firefox

[Anssi Kolehmainen]

Did some tests with webkit nightly builds:

r66130	broken
r64816	broken
r64727	broken
r64636	works
r64583	works
r64451	works

[Alexey Proskuryakov]

HTML5 tree builder (r64712) fits in this range, so it's a natural suspect.

[Adam Barth]

Thanks for the report.  I can reproduce.  Will investigate.

[Adam Barth]

Reduction:

<table>
  <tr><td>foobarbaz</td></tr>
  <tr><form><td>onetwothree</td></form></tr>
</table>

[Adam Barth]

I need to verify, but it looks like we get the same DOM as Firefox4, we just render it differently.

[Adam Barth]

Looks like the parsing of this case didn't change since Safari 5.  It might not be related to the parser or it might be some interaction between the parser and the rest of WebKit that we screwed up.

[Eric Seidel (no email)]

WTF.
http://trac.webkit.org/browser/trunk/WebCore/html/LegacyHTMLTreeBuilder.cpp?rev=64727#L428

[Adam Barth]

Apparently we're supposed to call HTMLFormElement::setDemoted.

[Eric Seidel (no email)]

Since Hyatt created setDemoted/isDemoted, he should probably see this bug go by.

[Adam Barth]

setDemoted has got be wrong.  If you build this same DOM with createElement/appendChild from script, I bet you get the wrong rendering.  We should probably use setDemoted in the first patch and then figure out the non-hacky solution in the second patch.

[Adam Barth]

Created attachment 65816 [details]
Patch

[Eric Seidel (no email)]

Comment on attachment 65816 [details]
Patch

WebCore/html/parser/HTMLConstructionSite.cpp:259
 +      ASSERT(element->tagQName() == formTag);
->hasName(formTag)

WebCore/html/parser/HTMLConstructionSite.cpp:256
 +  void HTMLConstructionSite::insertHTMLFormElement(AtomicHTMLToken& token, bool isDemoted)
isDemoted isn't very clear.  doNotAttach?  donno.

So wrong.  But better than what we had.

[Adam Barth]

Created attachment 65817 [details]
Patch for landing

[Adam Barth]

Comment on attachment 65817 [details]
Patch for landing

Clearing flags on attachment: 65817

Committed r66306: <http://trac.webkit.org/changeset/66306>

[Adam Barth]

All reviewed patches have been landed.  Closing bug.

[WebKit Commit Bot]

Comment on attachment 65817 [details]
Patch for landing

Rejecting patch 65817 from commit-queue.

Failed to run "[u'/Users/eseidel/Projects/CommitQueue/WebKitTools/Scripts/svn-apply', u'--force']" exit_code: 1
Last 500 characters of output:
/parser/HTMLConstructionSite.cpp
Hunk #1 FAILED at 253.
1 out of 1 hunk FAILED -- saving rejects to file WebCore/html/parser/HTMLConstructionSite.cpp.rej
patching file WebCore/html/parser/HTMLConstructionSite.h
Hunk #1 FAILED at 57.
1 out of 1 hunk FAILED -- saving rejects to file WebCore/html/parser/HTMLConstructionSite.h.rej
patching file WebCore/html/parser/HTMLTreeBuilder.cpp
Hunk #1 FAILED at 1154.
1 out of 1 hunk FAILED -- saving rejects to file WebCore/html/parser/HTMLTreeBuilder.cpp.rej

Full output: http://queues.webkit.org/results/3824056