1382102 - stylo: :nth-child() shouldn't match element inside XBL anonymous subtree

Status: RESOLVED FIXED

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

Description

[Xidorn Quan [:xidorn] UTC+10]

Test layout/style/test/test_selectors_on_anonymous_content.html checks that :nth-child() in the outer document doesn't match element inside XBL anonymous subtree.

Comment 1

[Xidorn Quan [:xidorn] UTC+10]

This seems to be XBL-related. TYLin, could you have a look?

Comment 2

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Attachment: style: Skip matching :nth-child if element is the root of anonymous subtree

This implement the logic in Gecko's nsNthIndexCache::GetNthIndex().

Review commit: https://reviewboard.mozilla.org/r/168738/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/168738/

Comment 3

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Attachment: Bug 1382102 - Update mochitest expectation.

Review commit: https://reviewboard.mozilla.org/r/168740/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/168740/

Comment 4

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=178a0bfb74909b70336819aa367d9293ba87313d

Comment 5

[Emilio Cobos Álvarez [:emilio]]

Comment on attachment 8897422 [details]
style: Skip matching :nth-child if element is the root of anonymous subtree

https://reviewboard.mozilla.org/r/168738/#review174164

r=me, with the method renamed. Thanks for fixing this!

::: servo/components/selectors/matching.rs:779
(Diff revision 1)
>                                     flags_setter: &mut F)
>                                     -> bool
>      where E: Element,
>            F: FnMut(&E, ElementSelectorFlags),
>  {
> +    if element.is_root_of_anonymous_subtree() {

I don't think we should not call `is_root_of_anonymous_subtree` to this.

Anon content is a very specific Gecko thing, and `selectors` is supposed to be a generic crate.

It's somewhat annoying because they shouldn't match even `:nth-child(1)`, sigh...

What about an `ignores_nth_child_selectors` kind of flag, that returns false by default, and returns `is_root_of_anonymous_subtree` in `gecko/wrapper.rs`?

I can't think of a really better name, but if you do find one, that'd be great :)

Comment 6

[Emilio Cobos Álvarez [:emilio]]

Comment on attachment 8897423 [details]
Bug 1382102 - Update mochitest expectation.

https://reviewboard.mozilla.org/r/168740/#review174168

Comment 7

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Comment on attachment 8897422 [details]
style: Skip matching :nth-child if element is the root of anonymous subtree

https://reviewboard.mozilla.org/r/168738/#review174164

> I don't think we should not call `is_root_of_anonymous_subtree` to this.
> 
> Anon content is a very specific Gecko thing, and `selectors` is supposed to be a generic crate.
> 
> It's somewhat annoying because they shouldn't match even `:nth-child(1)`, sigh...
> 
> What about an `ignores_nth_child_selectors` kind of flag, that returns false by default, and returns `is_root_of_anonymous_subtree` in `gecko/wrapper.rs`?
> 
> I can't think of a really better name, but if you do find one, that'd be great :)

> Anon content is a very specific Gecko thing, and selectors is supposed to be a generic crate.

You're right. It's better to keep `selectors` generic.

I think `ignores_nth_child_selectors` naming is good enough. Let's use that.

Comment 8

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Comment on attachment 8897423 [details]
Bug 1382102 - Update mochitest expectation.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/168740/diff/1-2/

Comment 9

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Attachment: Servo PR #18104

Comment 10

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

https://hg.mozilla.org/integration/autoland/rev/4ee1bb020c9f

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a531a8c8f75a
Update mochitest expectation. r=emilio

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/a531a8c8f75a