1483798 - getComputedStyle should return an empty style for elements outside the flattened tree

Status: RESOLVED FIXED

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

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

and this can be resolved as failing for now, since it matches other engines.

Comment 2

[Emilio Cobos Álvarez (:emilio)]

This is expected, for now, and matches other engines. I planned on fixing this as part of the effort of getting to a common getComputedStyle behavior across engines.

But I don't think this blocks shipping Shadow DOM since it matches them. I'll fix this once we ship Shadow DOM.

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

Attachment: getComputedStyle-detached-subtree.html.diff

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

oh, you had changed this bug while I was updating wpt results.

Comment 5

[Emilio Cobos Álvarez (:emilio)]

Works for me. I'll file another.

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

I'll make the testing changes elsewhere

Comment 7

[BugBot [:suhaib / :marco/ :calixte]]

There's a r+ patch which didn't land and no activity in this bug for 2 weeks.
:smaug, could you have a look please?

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

This landed as part of https://hg.mozilla.org/mozilla-central/rev/16daef3a89a2

Comment 9

[Emilio Cobos Álvarez (:emilio)]

The underlying issue (getComputedStyle outside of the flattened tree returning styles) is not fixed though.

Comment 10

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1483798 - Avoid returning the computed styles of nodes that aren't in the flat tree. r=#style,#layout-reviewers

As per spec. This could cause wrong styles to be returned because we
fill the bloom filter based on flat tree ancestors, which was ultimately
the root cause of bug 1737131.

Comment 11

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1483798 - Fix browser_ext_sidebarAction_context.js after the previous change. r=#extension-reviewers

This test was relying on querying the computed style of elements outside
of the tree (which could cause bogus results like bug 1737131).

In this case, it worked out for what the test wanted to assert, but
after the previous patch it no longer does (those elements effectively
do not have styles).

Fix the test in a similar fashion, to make sure we've opened all the
menus at least once so that we hit this code:

https://searchfox.org/mozilla-central/rev/42522adc3fbf4b98a3070d5aa5c56bee9c083ec4/toolkit/content/widgets/menupopup.js#88-96

and the elements are slotted.

Comment 12

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1483798 - Don't make menupopups and panels leave unslotted children. r=Gijs,mconley,bgrins

This is a more generic fix for
https://phabricator.services.mozilla.com/D131080 that we might want to
take instead. I'm not aware of other tests triggering the same issue but
this causes behavior to be a bit more predictable.

Context: We have tests that query the style of menupopups/menuitems that
are not in the flat tree (that is, that are children of a shadow host,
but are not slotted). That makes no sense, because the flat tree is how
we determine where to inherit from (so the styles that we were returning
before this bug are wrong, they just happen to work for that particular
test).

Panels and menupopups have some lazy initialization code that before
this patch left a shadow root attached to the element but no slot.
Instead, don't even bother to create a shadow root (and do that lazily
as well). It's a bit more predictable behavior, and is also less work
during startup, so should be a win.

Comment 13

[Phabricator Automation]

Comment on attachment 9250608 [details]
Bug 1483798 - Don't make menupopups and panels leave unslotted children. r=Gijs,mconley,bgrins

Revision D131081 was moved to bug 1741089. Setting attachment 9250608 [details] to obsolete.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5aa4d866a171
Avoid returning the computed styles of nodes that aren't in the flat tree. r=xidorn

Comment 15

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/5aa4d866a171