Bug 30232 – [HTML5] Add document.head

• Component: DOM
• Status: RESOLVED FIXED

[Joseph Pecoraro]

document.head is now in the HTML5 draft. It is in the Document's IDL:

Document IDL:
http://www.whatwg.org/specs/web-apps/current-work/#documents-in-the-dom

More Info on head accessor:
http://www.whatwg.org/specs/web-apps/current-work/#dom-tree-accessors

[Joseph Pecoraro]

Created attachment 40901 [details]
[PATCH] Adds document.head and Tests

[Joseph Pecoraro]

Created attachment 40902 [details]
[PATCH] Adds document.head and Tests (improved)

I made the equality tests stronger and fixed a style issue in the test case.

[Eric Seidel (no email)]

Comment on attachment 40902 [details]
[PATCH] Adds document.head and Tests (improved)

Seems wrong.  This already exists on HTMLDocument.idl.  Seems that we would either want to remove the one on HTMLDocument.idl if we did this, or that we don't want to do this at all.

The real change here is not for HTML docuemtns, but for XHTML documents! :)  Also, this would add .head for SVG documents.

[Darin Adler]

Let me say what Eric said, a little more emphatically.

The test attached to this bug already passes. Did you try it without your patch?

Making all of the HTMLDocument interface available to XML documents is something I think HTML 5 does call for in some fashion. I don't think it's urgent to move the head property from HTMLDocument  to Document as a separate step.

It's nice to have this test and we should land it, but *not* the code change.

[Joseph Pecoraro]

(In reply to comment #4)
> Let me say what Eric said, a little more emphatically.
> 
> The test attached to this bug already passes. Did you try it without your
> patch?

Yes I did test.  It fails in the current nightly! document.head is always undefined.  Eric was mistaken, it is not in HTMLDocument.idl.

> Making all of the HTMLDocument interface available to XML documents is
> something I think HTML 5 does call for in some fashion. I don't think it's
> urgent to move the head property from HTMLDocument  to Document as a separate
> step.

I see that I should have put it on HTMLDocument and not Document.  That is fixed in the new patch.  Eric also linked me to some newer resources on creating tests and I've attempted the newer style.

[Joseph Pecoraro]

Created attachment 40909 [details]
[PATCH] Add to HTMLDocument.idl and Newer Style Test

- HTMLDocument.idl not Document.idl
- Newer Style test with script-tests

[Darin Adler]

(In reply to comment #5)
> I see that I should have put it on HTMLDocument and not Document.

I'm not sure that's correct. I think perhaps it should be on Document. Is there a downside to having the head property be exposed to JavaScript in XML and SVG documents? If not, then you were right to have it in Document.

[Eric Seidel (no email)]

Comment on attachment 40909 [details]
[PATCH] Add to HTMLDocument.idl and Newer Style Test

OK, it would appear that document.head does not actually exist.  I'm surprised by this.  I thought it did.

I don't think this check is needed:
 var head = document.head;
 4 if (head)
 5     testPassed("document.head");

Given that document.body is defined in Document.idl instead of HTMLDocument.idl, I'm not sure that I was correct in telling you to move this to HTMLDocument.idl.

[Joseph Pecoraro]

> I'm not sure that's correct. I think perhaps it should be on Document. Is there
> a downside to having the head property be exposed to JavaScript in XML and SVG
> documents? If not, then you were right to have it in Document.

In order to track this down I checked to see if there was a downside to having the body property be exposed to JavaScript in XML and SVG.

From the Internet Side of Things:

This article (2005) seems to indicate that the web developer at the time did not expect document.body to appear in an XML or SVG Document and would fail in WebKit?
http://userjs.org/help/tips_and_tricks/what-type-of-page

Someone on the XML/SVG mailing list (2008) wrote that document.body was not a part of the SVG DOM:
http://article.gmane.org/gmane.text.xml.svg.devel/43755

document.body is not in the SVG 1.1 or SVG Tiny 1.2 Specs (under ECMAScript Bindings):
http://www.w3.org/TR/SVGTiny12/ecmascript-binding.html

However, on the WebKit side of things:

document.body was moved from HTMLDocument.idl to Document.idl in ChangeSet 26078 (2007):
http://trac.webkit.org/changeset/26078

The ChangeLog states:
> Moved a bunch of methods and properties from
> HTMLDocument down into Document to make
> them available for all documents (xml, svg).

Maciej Stachowiak on bug 12628 (2007):
> We should consider supporting all HTMLDocument properties and
> methods on all XML documents (because any XML document could
> contain XHTML).


I see no harm in putting the body and head properties on XML and SVG documents. Especially if document.body has been that way for over two years in WebKit without complaints.  Anyone know how Firefox handles this?

[Joseph Pecoraro]

> Anyone know how Firefox handles this?

On an SVG Document:
http://www.w3schools.com/svg/rect2.svg
http://grab.by/8sn

Firefox 3.5:
- document.body is undefined
- document.head is undefined (not supported)

WebKit Nightly:
- document.body is null (like the HTML5 spec says for HTML)
- document.head is undefined (as we are discussing)

[Joseph Pecoraro]

Should I CC someone in on this for a final decision?

(In reply to comment #8)
> I don't think this check is needed:
>    var head = document.head;
>    if (head)
>         testPassed("document.head");

You're right. I'll remove that in a new patch when a decision is made.

[Joseph Pecoraro]

Here is a clip from an IRC chat with othermaciej:

> that's a complicated question in theory, the absolutely correct thing
> is for it to go on HTMLDocument, and for us to make every single
> document implement all Document interfaces (including both
> HTMLDocument and SVGDocument) in practice, we don't have a
> way to do the "all interfaces on all documents" thing yet. thus, my
> suggestion would be that you should temporarily put it wherever
> document.body currently lives which happens to be Document
>
> though in the long term it should be on HTMLDocument and we
> should figure out how to do the HTML5 thing.

I agree with his suggestion. It would be easier to work with the attributes later if they are in the same place.

[Joseph Pecoraro]

Created attachment 40930 [details]
[PATCH] document.head in Document.idl

- Moved back to Document.idl.
- Removed the extra check in the script-test.

[Joseph Pecoraro]

Comment on attachment 40930 [details]
[PATCH] document.head in Document.idl

The ChangeLog's diff would make this fail in the commit-queue anyways. So it would need to be landed manually.

[Adam Barth]

Comment on attachment 40930 [details]
[PATCH] document.head in Document.idl

Based on the discussion on this bug and Maciej's quote from IRC, this patch appears correct.  Thanks!

[Joseph Pecoraro]

Landed in http://trac.webkit.org/changeset/49610
r49610 = a7e5c8e75b5404e71a0a7a6740f2b235f0b338f6

I had to provide a Windows Build Fix: r49611
Landed in http://trac.webkit.org/changeset/49611
Modeled after a previous Windows Build Fix for:
http://trac.webkit.org/changeset/48728

It appears as though there was an issue with Window's Code Generation.  Could someone enlighten me on why this was necessary, and if this is a problem could you point me in the right direction for getting this fixed?  Is there someone who knows a lot about Window's code generation?

The error I was received was:
http://build.webkit.org/builders/Windows%20Debug%20%28Build%29/builds/6292/steps/compile-webkit/logs/stdio
GEN_DOMHTMLEmbedElement.cpp
..\DOMCreateInstance.cpp(207) : error C2259: 'GEN_DOMHTMLDocument' : cannot instantiate abstract class
        due to following members:
        'HRESULT IGEN_DOMDocument::head(IGEN_DOMHTMLHeadElement **)' : is abstract
        C:\cygwin\home\buildbot\slave\win-release\build\WebKitBuild\Include\WebKit/WebKit.h(61160) : see declaration of 'IGEN_DOMDocument::head'

After adding the build fix Windows passed all the Layout tests, including my newly added test, which confused me, but maybe that is how it should be?

Sorry about all this for a one line fix!