Bug 43758 - HTML TreeBuilder hits ASSERT in fragment case with insertAdjacentHTML and colgroup
Summary: HTML TreeBuilder hits ASSERT in fragment case with insertAdjacentHTML and col...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-08-09 16:53 PDT by Eric Seidel (no email)
Modified: 2010-08-11 20:13 PDT (History)
3 users (show)

See Also:


Attachments
Patch (7.00 KB, patch)
2010-08-11 15:14 PDT, Adam Barth
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Seidel (no email) 2010-08-09 16:53:20 PDT
HTML TreeBuilder hits ASSERT in fragment case with insertAdjacentHTML and colgroup

ASSERTION FAILED: isEmpty()
(/Projects/WebKit/WebCore/html/HTMLTreeBuilder.cpp:299 WebCore::HTMLTreeBuilder::ExternalCharacterTokenBuffer::~ExternalCharacterTokenBuffer())

Test case:

<div id="test"></div>
<script>
testZone = document.getElementById('test');
var colgroup = document.createElement("colgroup");
testZone.appendChild(colgroup);
colgroup.insertAdjacentHTML("beforeBegin", "HTML");
</script>


Not sure why yet.
Comment 1 Eric Seidel (no email) 2010-08-09 17:00:36 PDT
OK, the bug is that HTMLElement::insertAdjacentHTML is passing the wrong contextElement.

It's passing the colgroup (this) instead of the div.  It's possible that insertAdjacentHTML("afterBegin", "HTML") would hit this ASSERT too, but we need to fix the context element bug first.

This bug has been around for forever, since the xml parser was always passed the wrong context.  The design is wrong, because we can't create the DocumentFragment before we know where we're inserting it.
Comment 2 Eric Seidel (no email) 2010-08-09 17:54:04 PDT
Ok, I've found and fixed one bug, but this still hits the ASSERT:

var colgroup = document.createElement("colgroup");
shouldBeUndefined("colgroup.insertAdjacentHTML('afterBegin', 'text')");
Comment 3 Eric Seidel (no email) 2010-08-10 01:05:36 PDT
It appears the spec wants to drop character tokens on the floor:

http://www.whatwg.org/specs/web-apps/current-work/multipage/tokenization.html#parsing-main-incolgroup

Anything else
Act as if an end tag with the tag name "colgroup" had been seen, and then, if that token wasn't ignored, reprocess the current token.

The fake end tag token here can only be ignored in the fragment case.


We don't have any way to "ignore" character tokens currently.  We could make one.  But this may also be a spec bug.

The reduced test case to ASSERT is:

document.createElement("colgroup").innerHTML = "foo";
Comment 4 Adam Barth 2010-08-11 14:45:18 PDT
I'd like to link to the spec bug you filed, but I can't seem to find it.
Comment 5 Adam Barth 2010-08-11 15:14:08 PDT
Created attachment 64167 [details]
Patch
Comment 6 Eric Seidel (no email) 2010-08-11 17:04:19 PDT
Comment on attachment 64167 [details]
Patch

I have not filed a bug about the colgroup issue, no.
Comment 7 Eric Seidel (no email) 2010-08-11 17:33:53 PDT
Comment on attachment 64167 [details]
Patch

We need to fix the createContextualFragment nonsense.
Comment 8 WebKit Commit Bot 2010-08-11 20:13:37 PDT
Comment on attachment 64167 [details]
Patch

Clearing flags on attachment: 64167

Committed r65213: <http://trac.webkit.org/changeset/65213>
Comment 9 WebKit Commit Bot 2010-08-11 20:13:42 PDT
All reviewed patches have been landed.  Closing bug.