1355715 - stylo: test_namespace_rule.html fails for several cases

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, enhancement, P2)

Description

[Xidorn Quan [:xidorn] UTC+11]

There are 17 failures in layout/layout/test/test_namespace_rule.html, and it seems to me they are probably just bugs in how Servo handles @namespace rule, and this test doesn't use much the CSSOM namespace rule.

Comment 1

[Xidorn Quan [:xidorn] UTC+11]

Attachment: testcase

So there are 6 failures left, which all have empty namespace uri involves. See the testcase.

I suspect these failures are all because we use empty atom for kNameSpaceID_None rather than an independent namespace as mentioned in comment inside nsNameSpaceManager.h [1] and bug 1292278 comment 2.

I'm not sure what should we do for the failures here. All the failing tests were imported from Opera's test suite, and all other browsers pass them. Empty namespace may not really lead to webcompat issue nowadays, but given that Gecko's current behavior is quite interop with others, I'm somehow unwilling to see it gets changed with Stylo.

bholley, what do you think?


[1] https://searchfox.org/mozilla-central/rev/5dadcbe55b4ddd1e448c06c77390ff6483aa009b/dom/base/nsNameSpaceManager.h#52-53

Comment 2

[Simon Sapin (:SimonSapin)]

> we use empty atom for kNameSpaceID_None rather than an independent namespace

I believe this behavior is correct per spec. In every entry point (the XML parser, DOM APIs, CSS @namespace rules), using an empty string for a namespace URL is either an error or is normalized to "no namespace" a.k.a. "the null namespace". Relevant to the attached test case:

https://www.w3.org/TR/REC-xml-names/#defaulting
> If there is no default namespace declaration in scope, the namespace name has no value.
> […]
> The attribute value in a default namespace declaration MAY be empty. This has the same effect, within the scope of the declaration, of there being no default namespace. 

https://drafts.csswg.org/css-namespaces/#terminology
> In CSS Namespaces a namespace name consisting of the empty string is taken to represent the null namespace or lack of a namespace.

Comment 3

[Xidorn Quan [:xidorn] UTC+11]

So, that may be correct per spec, and in the attached testcase, replacing "foo|test" with "|test" also gets a green rect in all browsers, which indicates that "foo" namespace really just means null namespace in browsers.

If empty string in xmlns attribute also indicates null namespace, I guess getting a green rect from the test case is the correct behavior per spec.

Comment 4

[Bobby Holley (:bholley)]

(In reply to Xidorn Quan [:xidorn] UTC+10 from comment #1)
> I'm not sure what should we do for the failures here. All the failing tests
> were imported from Opera's test suite, and all other browsers pass them.
> Empty namespace may not really lead to webcompat issue nowadays, but given
> that Gecko's current behavior is quite interop with others, I'm somehow
> unwilling to see it gets changed with Stylo.
> 
> bholley, what do you think?

If all the UAs currently agree and disagree with the spec, then nobody has demonstrated that the spec is web-compatible, and stylo is not the right vehicle to do that. Let's align with Gecko and the other UAs.

Comment 5

[Bobby Holley (:bholley)]

(In reply to Bobby Holley (:bholley) (busy with Stylo) from comment #4)
> If all the UAs currently agree and disagree with the spec

To be precise, I meant "agree with each other and jointly disagree with the spec".

Comment 6

[Simon Sapin (:SimonSapin)]

That is not the case here as far as I can tell. There is interop and implementations agree with the spec and this test. It’s only Stylo that has a bug somewhere, but I believe that bug is not "empty string namespace should be distinguished from no namespace" as Xidorn suggested earlier.

Comment 7

[Boris Zbarsky [:bzbarsky]]

All the UAs agree with each other an with the spec here.  "" means null namespace.  Per spec, the square in https://bug1355715.bmoattachments.org/attachment.cgi?id=8885133 should be green, and is in browsers.

It's possible the problem lies in whatever code maps servo namespace strings to Gecko namespace ids?

Comment 8

[Boris Zbarsky [:bzbarsky]]

Note that servo gets a green square here too, so this is really pretty stylo-specific.

Comment 9

[Xidorn Quan [:xidorn] UTC+11]

I know what's going on...

Comment 10

[Xidorn Quan [:xidorn] UTC+11]

Attachment: Bug 1355715 - Use empty atom rather than 'empty' atom for none namespace.

Review commit: https://reviewboard.mozilla.org/r/157274/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/157274/

Comment 11

[Bobby Holley (:bholley)]

Comment on attachment 8886481 [details]
Bug 1355715 - Use empty atom rather than 'empty' atom for none namespace.

https://reviewboard.mozilla.org/r/157274/#review162408

Doh! r=me

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/bd8b78472a65
Use empty atom rather than 'empty' atom for none namespace. r=bholley

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/bd8b78472a65