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)

defect
Not set
normal

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;
};
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 ;-)
Attached file Testcase
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.
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/
(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.
Keywords: regression
OS: Linux → All
Hardware: x86 → All
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
(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.
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?
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.
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.

Attachment

General

Creator:
Created:
Updated:
Size: