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)

53 Branch
enhancement

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.
Priority: -- → P2
Assignee: nobody → ferjmoreno
Assignee: ferjmoreno → nobody
Attached file 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
Flags: needinfo?(bobbyholley)
> 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.
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.
(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)
(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".
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.
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?
Note that servo gets a green square here too, so this is really pretty stylo-specific.
I know what's going on...
Assignee: nobody → xidorn+moz
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+
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
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: