Bug 99244 – According to DOM4, all DocType nodes should have a document

• Status: RESOLVED
• Resolution: FIXED
• Component: DOM

[Adam Barth]

According to DOM4, all DocType nodes should have a document

[Adam Barth]

Created attachment 168546 [details]
Work in progress

[Adam Barth]

Once this lands, this will simply a ton of special cases related to DocType nodes throughout WebCore.

[Alexey Proskuryakov]

I suggest waiting until (and if) DOM 4 gains any official capacity. This is a direct violation of each and every official specification, and a working draft cannot override that.

[Alexey Proskuryakov]

*** Bug 120425 has been marked as a duplicate of this bug. ***

[Chris Dumez]

Alexey, based on your comment in the duplicate bug, I assume you are still not in favor of making this change?

[Alexey Proskuryakov]

I think that it's crazy and destructive to change specs like this. I don't have much of an opinion on whether WebKit should accept this change at this point in time.

[Darin Adler]

We can easily remove the tons of special cases in WebCore another way. There’s no harm in having the DocumentType have a document that is not visible to the web through the ownerDocument attribute. And document() can return that document.

[Antti Koivisto]

We should do this. It fixes a hole in platform consistency and is obscure enough that it has virtually zero probability of breaking anything.

[Chris Dumez]

I would be happy to port my Blink patch if the approach (same as DOM4) is accepted for WebKit:
https://src.chromium.org/viewvc/blink?revision=156581&view=revision

Let me know.
I haven't done so yet because of Darin and Alexey's comments.

[Chris Dumez]

(In reply to comment #9)
> I would be happy to port my Blink patch if the approach (same as DOM4) is accepted for WebKit:
> https://src.chromium.org/viewvc/blink?revision=156581&view=revision
> 
> Let me know.
> I haven't done so yet because of Darin and Alexey's comments.

As explained in my duplicate bug, Firefox stable already ships with the behavior specified in DOM4.

[Andreas Kling]

I think we should make this change. The invariant of every Node always has a Document is very valuable, and like Antti, I doubt anyone relies on this quirk.

In fact, even if DOM4 hadn't changed, I *still* think we should be making the change.

[Darin Adler]

Yes, I think fixing this DocumentType silliness is helpful and highly unlikely to be a compatibility problem despite the years of having a standard clearly stating it should work that way.

[Darin Adler]

Don’t let my comment stand in your way!

[Chris Dumez]

Created attachment 210010 [details]
Patch

[Chris Dumez]

Comment on attachment 210010 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=210010&action=review

> Source/WebCore/dom/Node.h:404
>      Document* document() const

I guess it would be nice to return a reference now. I would be happy to do that in a follow-up patch unless someone knows of a reason why we shouldn't.

[Andreas Kling]

(In reply to comment #15)
> (From update of attachment 210010 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=210010&action=review
> 
> > Source/WebCore/dom/Node.h:404
> >      Document* document() const
> 
> I guess it would be nice to return a reference now. I would be happy to do that in a follow-up patch unless someone knows of a reason why we shouldn't.

I already have a draft monster patch for doing this (it'll be >1MB) so you shouldn't waste your time doing the same thing. :)

[Chris Dumez]

Comment on attachment 210010 [details]
Patch

Thanks for reviewing.

[WebKit Commit Bot]

Comment on attachment 210010 [details]
Patch

Clearing flags on attachment: 210010

Committed r154840: <http://trac.webkit.org/changeset/154840>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Lucas Forschler]

Mass moving XML DOM bugs to the "DOM" Component.