Closed Bug 1320000 Opened 3 years ago Closed 3 years ago
Element::Parse Attribute doesn't delegate to its superclass
XBLChildrenElement::ParseAttribute doesn't call nsXMLElement::ParseAttribute at the end, which means when we set class="" on an <xbl:children> element, we don't end up at Element::ParseAttribute, which means (a) we don't set NODE_MAY_HAVE_CLASS on the element, and (b) we end up storing a single class name as a string in the nsAttrValue, rather than an nsIAtom, in Element::SetAttr. This invalidates our assertion in ClassOrClassList in ServoBindings.cpp that if the class="" attribute has a string in the nsAttrValue, that it must be pure white space.
Assignee: nobody → cam
Status: NEW → ASSIGNED
Attachment #8813962 - Flags: review?(mrbkap)
Attachment #8813962 - Attachment is patch: true
layout/reftests/dom/xbl-children-2.xhtml now fails, with a single "FAIL" showing, which I assume is the <xbl:children class="forced"> element. Those elements are normally not displayed due to this rule: http://searchfox.org/mozilla-central/rev/feef954874af9a18168e61a75629a9406b847c53/toolkit/content/xul.css#53 Since they're just being treated as regular elements here, I guess it's not a problem to allow this. (Oddly, when I load that test in a regular browser tab, all four FAILs show. My guess is that's because xul.css is loaded lazily, and we don't trigger loading it based on an XBL-namespaced element. Nearly all the other rules in that file are for XUL elements.)
Comment on attachment 8813997 [details] [diff] [review] patch v2 As much as it doesn't make sense to set |display: block| on an <xbl:children> element, I don't think we should allow that to take effect. Can we make the xbl|children rule be |display: none !important;| instead?
Attachment #8813997 - Flags: review?(mrbkap) → review-
Attachment #8815545 - Flags: review?(mrbkap) → review+
Comment on attachment 8815546 [details] [diff] [review] Part 2: Make XBLChildrenElement::ParseAttribute delegate to its superclass. Review of attachment 8815546 [details] [diff] [review]: ----------------------------------------------------------------- Thanks.
Attachment #8815546 - Flags: review?(mrbkap) → review+
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/mozilla-inbound/rev/bee6662585ed Part 1: Force xbl:children elements to be display:none. r=mrbkap https://hg.mozilla.org/integration/mozilla-inbound/rev/523009ae06f6 Part 2: Make XBLChildrenElement::ParseAttribute delegate to its superclass. r=mrbkap
You need to log in before you can comment on or make changes to this bug.