Bug 149104 – Node.baseURI should not return null for detached nodes

• Component: DOM
• Status: RESOLVED FIXED

[Chris Dumez]

Node.baseURI should not return null for detached nodes. It should return the node document's base URL. The node document is set when the node is created so it is valid even if the node is detached:
https://dom.spec.whatwg.org/#dom-node-baseuri

WebKit currently traverses the parents to find the base URL, which only works if the node is attached. Also, WebKit takes into account the xml:base attribute when computing the baseURI.

Both Chrome and Firefox already dropped support for xml:base so I think we should as well:
https://code.google.com/p/chromium/issues/detail?id=341854
https://bugzilla.mozilla.org/show_bug.cgi?id=903372

Firefox complies with the specification. Chrome's baseURI still only works for attached Nodes as their implementation still traverses the DOM tree, despite dropping support for xml:base.

[Chris Dumez]

Relevant W3C bug: https://www.w3.org/Bugs/Public/show_bug.cgi?id=20976

[Chris Dumez]

rdar://problem/22559535

[Chris Dumez]

Created attachment 261083 [details]
Patch

[Sam Weinig]

Comment on attachment 261083 [details]
Patch

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

> LayoutTests/dom/xhtml/level3/core/nodegetbaseuri03-expected.txt:3
> +Message	nodegetbaseuri03: assertNull failed, actual file:///Users/chris/WebKit/OpenSource/LayoutTests/dom/xhtml/level3/core/nodegetbaseuri03.xhtml

I don't think we want to check-in something with your user specific path in it.

[Chris Dumez]

Comment on attachment 261083 [details]
Patch

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

>> LayoutTests/dom/xhtml/level3/core/nodegetbaseuri03-expected.txt:3
>> +Message	nodegetbaseuri03: assertNull failed, actual file:///Users/chris/WebKit/OpenSource/LayoutTests/dom/xhtml/level3/core/nodegetbaseuri03.xhtml
> 
> I don't think we want to check-in something with your user specific path in it.

Interesting :) sorry about that, I guess I will have to tweak the test.

[Build Bot]

Comment on attachment 261083 [details]
Patch

Attachment 261083 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/168031

New failing tests:
dom/xhtml/level3/core/nodegetbaseuri03.xhtml

[Build Bot]

Created attachment 261085 [details]
Archive of layout-test-results from ews103 for mac-mavericks

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-mavericks  Platform: Mac OS X 10.9.5

[Build Bot]

Comment on attachment 261083 [details]
Patch

Attachment 261083 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/168035

New failing tests:
dom/xhtml/level3/core/nodegetbaseuri03.xhtml

[Build Bot]

Created attachment 261086 [details]
Archive of layout-test-results from ews104 for mac-mavericks-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-mavericks-wk2  Platform: Mac OS X 10.9.5

[Chris Dumez]

Created attachment 261089 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 261089 [details]
Patch

Clearing flags on attachment: 261089

Committed r189677: <http://trac.webkit.org/changeset/189677>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Darin Adler]

Comment on attachment 261089 [details]
Patch

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

> LayoutTests/dom/xhtml/level3/core/nodegetbaseuri03.js:102
> -      assertNull("nodegetbaseuri03",baseURI);
> +      assertTrue("nodegetbaseuri03", baseURI == null);

Why change this if it’s going to fail anyway? We should consider just removing the test entirely if we’re going to make changes like these. Or changing the test to expect correct behavior.

[Chris Dumez]

(In reply to comment #13)
> Comment on attachment 261089 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=261089&action=review
> 
> > LayoutTests/dom/xhtml/level3/core/nodegetbaseuri03.js:102
> > -      assertNull("nodegetbaseuri03",baseURI);
> > +      assertTrue("nodegetbaseuri03", baseURI == null);
> 
> Why change this if it’s going to fail anyway? We should consider just
> removing the test entirely if we’re going to make changes like these. Or
> changing the test to expect correct behavior.

These reason I had to change it is because when it failed, it was printing out an absolute path and therefore would give different results on the bots. I otherwise try not to edit imported compliance tests as I think it is our policy.

I however agree with you that we could simply drop some of the old DOM compliance tests that are now outdated. I'll do that from now on and drop this one in a follow-up patch.