Closed
Bug 1355715
Opened 8 years ago
Closed 7 years ago
stylo: test_namespace_rule.html fails for several cases
Categories
(Core :: CSS Parsing and Computation, enhancement, P2)
Tracking
()
RESOLVED
FIXED
mozilla56
Tracking | Status | |
---|---|---|
firefox56 | --- | fixed |
People
(Reporter: xidorn, Assigned: xidorn)
References
Details
Attachments
(2 files)
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.
Updated•8 years ago
|
Priority: -- → P2
Updated•8 years ago
|
Assignee: nobody → ferjmoreno
Updated•8 years ago
|
Assignee: ferjmoreno → nobody
Assignee | ||
Comment 1•7 years ago
|
||
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
Flags: needinfo?(bobbyholley)
Comment 2•7 years ago
|
||
> 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.
Assignee | ||
Comment 3•7 years ago
|
||
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•7 years ago
|
||
(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.
Flags: needinfo?(bobbyholley)
Comment 5•7 years ago
|
||
(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•7 years ago
|
||
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•7 years ago
|
||
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•7 years ago
|
||
Note that servo gets a green square here too, so this is really pretty stylo-specific.
Comment hidden (mozreview-request) |
Comment 11•7 years ago
|
||
mozreview-review |
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
Attachment #8886481 -
Flags: review?(bobbyholley) → review+
Comment 12•7 years ago
|
||
Pushed by xquan@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/bd8b78472a65
Use empty atom rather than 'empty' atom for none namespace. r=bholley
Comment 13•7 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox56:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
You need to log in
before you can comment on or make changes to this bug.
Description
•