Fix #1548. #2563
Fix #1548. #2563
Conversation
Additional resolution in w3c#1548.
cssom-1/Overview.bs
Outdated
| and its <a>shadow-including root</a>'s <a>browsing context</a> either doesn't have a | ||
| <a>browsing context container</a>, or that <a>browsing context container</a> | ||
| is <a>being rendered</a>, | ||
| let <var>decls</var> be a list of all longhand properties that are |
There was a problem hiding this comment.
You want to use set here, since you're overwriting something. See https://infra.spec.whatwg.org/. (Older specifications are inconsistent with this unfortunately.)
There was a problem hiding this comment.
Alright, thanks for the tip, will change :)
cssom-1/Overview.bs
Outdated
| <li>If <var>elt</var> is <a>connected</a>, part of the <a>flat tree</a>, | ||
| and its <a>shadow-including root</a>'s <a>browsing context</a> either doesn't have a | ||
| <a>browsing context container</a>, or that <a>browsing context container</a> | ||
| is <a>being rendered</a>, |
There was a problem hiding this comment.
When would it (not) be rendered?
There was a problem hiding this comment.
When it's a display: none iframe, for example (IIUC), or one of the ancestors in the chain is not rendered itself, which is one of the cases we wan to return no styles in.
cssom-1/Overview.bs
Outdated
| @@ -2779,19 +2782,27 @@ steps: | |||
| versions. That is, both <code>:before</code> and <code>::before</code> should | |||
| match above. | |||
|
|
|||
| <li>Let <var>decls</var> be an empty list of <a>CSS declarations</a>. | |||
| <li>If <var>elt</var> is <a>connected</a>, part of the <a>flat tree</a>, | |||
| and its <a>shadow-including root</a>'s <a>browsing context</a> either doesn't have a | |||
There was a problem hiding this comment.
What if the root doesn't have a browsing context?
There was a problem hiding this comment.
Could you put an example of a document which has no browsing context? It's not clear to me when can that happen.
There was a problem hiding this comment.
This is why I asked you for review btw, thanks a lot for taking a look :-)
There was a problem hiding this comment.
new Document(), xhr.responseXML, new DOMParser().parseFromString(...), ...
There was a problem hiding this comment.
Alright, I think we don't want to return styles there either, since those documents have no rendering behavior. Pushed two fixups for the issues you raised, mind taking a look? Thanks!
|
This looks fine to me, editorially and behaviorally, but I do wonder whether it matches implementations. In my own testing many years ago I've found that It's probably worth accompanying this change with web-platform-tests. (I don't know if the CSS WG has such a policy yet, but at the WHATWG each normative change to a standard requires corresponding tests and bugs against failing implementations.) |
|
Yeah, we decided this in #1548. This is kind of a mess in terms of interop right now. I added tests for this in web-platform-tests/wpt#10422. |
|
Basically, this is where we want to move. It may not be possible to not return styles in display: none iframes because of web compat, but we decided to give it a shot. |
|
Cool. I'd generally wait for at least one browser to ship before committing on the change in model, but seems like most things (apart from browser bugs) are covered here. |
* [cssom] Make elements outside of the flat tree or detached not expose styles. Fixes w3c#1548 * [cssom] Let elements in non-rendered iframes not expose styles. Additional resolution in w3c#1548. * fixup! [cssom] Make elements outside of the flat tree or detached not expose styles. * fixup! [cssom] Let elements in non-rendered iframes not expose styles.
See individual commits for reference.