stylo: preshints (mapped attributes) not handled correctly for elements created from non-docshell-associated document
Categories
(Core :: CSS Parsing and Computation, enhancement, P3)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox57 | --- | wontfix |
People
(Reporter: xidorn, Unassigned)
References
Details
From bug 1353177 comment 5, an example code which passes in Gecko but fails in Stylo: <!DOCTYPE html> <body> <script> document.body.offsetWidth; var doc = document.implementation.createHTMLDocument(); var f = doc.createElement("font"); f.setAttribute("color", "green"); alert(getComputedStyle(f).color); </script> </body> A poc patch is submitted in bug 1353177 comment 7, but it is not clear whether that is the right way forward.
|
Reporter | |
Updated•6 years ago
|
|
||
Updated•6 years ago
|
|
|
Reporter | |
Comment 1•6 years ago
|
||
(In reply to Boris Zbarsky [:bz] (still digging out from vacation mail) from bug 1353177 comment #8) > Hmm. Doing the "pass the document" thing makes some sense. I'm not > entirely sure about the way it picks a prescontext to use and whether that > approach makes sense. > > How does Gecko pass my testcase from bug 1353177 comment 6 without creating > an attr stylesheet? I guess it just has a non-sheet-associated mapped > attributes that it then walks without involving an attr stylesheet in any way? So the Gecko counterpart of ServoSpecifiedValues is nsRuleData, which is created in-stack in nsRuleNode::WalkRuleTree [1] with the pres context of the rule node. I didn't track all way up for where is the nsRuleNode comes from, but from a debugger, it seems to me the nsRuleNode is from the document of the nsComputedDOMStyle, which is different from the owner document of the target element. Given that, my guess is that Gecko always uses the pres context nsComputedDOMStyle. We can probably do so for Stylo as well. [1] https://searchfox.org/mozilla-central/rev/15ce5cb2db0c85abbabe39a962b0e697c9ef098f/layout/style/nsRuleNode.cpp#2583-2584
|
|
Reporter | |
Comment 2•6 years ago
|
||
(In reply to Boris Zbarsky [:bz] (still digging out from vacation mail) from bug 1353177 comment #8) > > I suspect that we should probably create mAttrStyleSheet when document is created. > > It's not clear which is best here. This would involve allocating a bunch of > stuff... but maybe we allocate it anyway in common "data document" cases > like XHR result documents. Probably we can create it lazily, e.g. creating one when nsIDocument::GetAttributeStyleSheet is called (and rename it to nsIDocument::AttributeStyleSheet instead), so that for document we don't need to calculate stylesheet, we won't need it.
|
||
Comment 3•6 years ago
|
||
status-firefox57=wontfix unless someone thinks this bug should block 57
|
|
||
Comment 4•4 years ago
|
||
This is still failing. The original testcase no longer shows the problem due to disconnected elements no longer having useful computed style, but this testcase does:
<!DOCTYPE html>
<body>
<script>
document.body.offsetWidth;
var doc = document.implementation.createHTMLDocument();
var f = doc.createElement("font");
doc.body.appendChild(f);
f.setAttribute("color", "green");
alert(getComputedStyle(f).color);
</script>
</body>
That alerts rgb(0, 0, 0) in Gecko (now with stylo), empty string in Chrome, rgb(0, 128 ,0) in Safari.
I wonder whether we could get the spec to specify a simple behavior here that people could align on, given the lack of compat.
|
||
Comment 5•4 years ago
|
||
That test-case is also using getComputedStyle from a different window... Per spec Safari is right, and we'd match it if we fixed bug 1471231 I suspect (since we'd be able to compute the style there).
But we can probably get away specifying the Chrome behavior too for this case, if we wanted. And should be somewhat easy to implement as long as it's easy to distinguish from the display: none iframe case.
|
|
||
Comment 6•4 years ago
|
||
That test-case is also using getComputedStyle from a different window...
Well, sort of. There's only one window involved here; doc does not have a window at all.
|
||
Updated•6 months ago
|
Description
•