Closed
Bug 521345
Opened 15 years ago
Closed 15 years ago
[HTML] createContextualFragment on a range whose endpoints are children of the document creates head and body
Categories
(Core :: DOM: HTML Parser, defect)
Core
DOM: HTML Parser
Tracking
()
RESOLVED
INVALID
People
(Reporter: johnjbarton, Unassigned)
References
Details
(Keywords: regression)
Attachments
(2 files)
This bug was discovered by Boris and got confused with
Bug 520421 - Firebug execution line UI update fails
In Firebug we do an appendInnerHTML of the following:
var paddedSource =
"<div class='topSourcePadding'>" +
"<div class='sourceRow'><div class='sourceLine'></div><div class='sourceRowText'></div></div>"+
"</div>"+
"<div class='sourceViewport'></div>"+
"<div class='bottomSourcePadding'>"+
"<div class='sourceRow'><div class='sourceLine'></div><div class='sourceRowText'></div></div>"+
"</div>";
and we get in this:
<div class=" sourceBox" collapsed="true"><head></head><body><div class="topSourcePadding"><div class="sourceRow"><div class="sourceLine"></div><div class="sourceRowText"></div></div></div><div class="sourceViewport"></div><div class="bottomSourcePadding"><div class="sourceRow"><div class="sourceLine"></div><div class="sourceRowText"></div></div></div></body></div>
the code for our appendInnerHTML is
this.appendInnerHTML = function(element, html, referenceElement)
{
var doc = element.ownerDocument;
var range = doc.createRange();
range.selectNode(doc.body);
var fragment = range.createContextualFragment(html);
var firstChild = fragment.firstChild;
element.insertBefore(fragment, referenceElement);
return firstChild;
};
|
Reporter | |
Comment 1•15 years ago
|
||
Our call is
appendInnerHTML(sourceBox, paddedSource);
where sourceBox is
var sourceBox = this.document.createElement("div");
and paddedSource is above.
This is all HTML so even though the test case is not here yet, the ingredients are ;-)
|
|
Reporter | |
Comment 2•15 years ago
|
||
|
||
Comment 3•15 years ago
|
||
Without the HTML5 parser, the alerts are: 1, HTMLDivElement, undefined
With the HTML5 parser, the alerts are: 2, HTMLHeadElement, HTMLBodyElement
With the HTML5 parser, the <div> is a child of the <body> in the resulting DOM.
The old behavior seems pretty clearly broken to me (with Firebug relying on the brokenness, apparently): parsing the string "<div></div>" as a child of a document (which is what the fragment context is), should in fact be inferring a <head> and <body> in HTML. Certainly the non-fragment parser does so; the fact that the fragment one wasn't was just an oversight.
|
|
||
Comment 4•15 years ago
|
||
Leaving for Henri to look at, but this looks invalid to me.
Summary: [html5] extraneous tags created → [HTML] createContextualFragment on a range whose endpoints are children of the document creates head and body
It seems that invoking the HTML5 fragment parsing algorithm without a range context (as opposed to an element context) is undefined in the spec.
However, what happens here isn't what I expected to happen per the code I wrote, so I need to look at this in a debugger.
s/without/with/
|
||
Comment 7•15 years ago
|
||
(In reply to comment #0)
> This bug was discovered by Boris and got confused with
> Bug 520421 - Firebug execution line UI update fails
Magic name change. :) Wasn't it found by myself? :)
As given on bug 520421 this has been regressed when the html5 parser has been landed. So I will add the dependency to bug 487949.
When you select body, the context node is its parent: html. When the HTML5 fragment parsing algorithm is invoked with "html" as the context, head and body are inferred.
Hence, I'm marking this as invalid.
I suggest saying range.selectNode(referenceElement); instead of range.selectNode(doc.body);.
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → INVALID
|
|
Reporter | |
Comment 9•15 years ago
|
||
(In reply to comment #8)
> When you select body, the context node is its parent: html. When the HTML5
> fragment parsing algorithm is invoked with "html" as the context, head and body
> are inferred.
>
> Hence, I'm marking this as invalid.
>
> I suggest saying range.selectNode(referenceElement); instead of
> range.selectNode(doc.body);.
However most calls, including the one in the test case, have no referenceElement.
|
|
||
Comment 10•15 years ago
|
||
Don't you _actually_ want to position the range where you will be inserting? That is, immediately before referenceElement (so at the end if there isn't one) in the child list of |element|? That is, if you're trying to pretend that the HTML was parsed at that spot (which it sounds like you are), then that's the context you want to be using, no?
|
|
||
Comment 11•15 years ago
|
||
Something along these lines:
if (referenceElement) {
range.setStartBefore(referenceElement);
range.setEndBefore(referenceElement);
} else {
range.setStart(element, element.childnodes.length);
range.setEnd(element, element.childnodes.length);
}
Though in practice, I bet just |range.selectNodeContents(element)| will have the equivalent effect, since I don't think createContextualFragment actually takes siblings into account.
|
|
||
Comment 12•15 years ago
|
||
And in particular, all it cares about is the range's start parent, fwiw.
You need to log in
before you can comment on or make changes to this bug.
Description
•