Open Bug 1400771 Opened 7 years ago Updated 2 years ago

stylo: preshints (mapped attributes) not handled correctly for elements created from non-docshell-associated document

Categories

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

Product:
Core
Component:
CSS Parsing and Computation
Type:
enhancement

Tracking

()

Tracking Flags:
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.
Priority: -- → P3
Summary: stylo: preshints (mapped attributes) not handled correctly for elements created from different document → stylo: preshints (mapped attributes) not handled correctly for elements created from non-docshell-associated document
(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
(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.
status-firefox57=wontfix unless someone thinks this bug should block 57
status-firefox57: affected → wontfix
Depends on: 1428339

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.

Flags: needinfo?(emilio)

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.

Flags: needinfo?(emilio)

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.

Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.