1372876 - stylo: Use element instead of rule_hash_target when calling get_declarations_from_xbl_bindings()

Status: RESOLVED FIXED

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

Description

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

File per bug 1371577 comment 9.

::: servo/components/style/stylist.rs:1091
(Diff revision 1)
>              debug!("skipping user rules");
>          }
>  
>          // Step 3b: XBL rules.
>          let cut_off_inheritance =
> -            rule_hash_target.get_declarations_from_xbl_bindings(applicable_declarations);
> +            rule_hash_target.get_declarations_from_xbl_bindings(pseudo_element,

Oh, I think I know why that is, could you file a bug and ni? me for that? I'll clean that up.

Comment 1

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Emilio, ni? you per your request. (I don't know a proper bug title. You might want to change that.)

Comment 2

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Attachment: stylo: Fix pseudo element matching for rules in XBL stylesheets (bug 1372876)

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

Comment 3

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Emilio, does this patch match what you thought?

With this patch, the NAC children like ::-moz-range-progress generated by <input type="range> in <video controls> [1] can now have rules matched in XBL stylesheet [2].

[1] http://searchfox.org/mozilla-central/rev/ae94cfb36d804727dfb38f2750bfcaab4621df6e/toolkit/content/widgets/videocontrols.xml#177
[2] http://searchfox.org/mozilla-central/rev/ae94cfb36d804727dfb38f2750bfcaab4621df6e/toolkit/themes/shared/media/videocontrols.css#284-296

Comment 4

[Emilio Cobos Álvarez [:emilio]]

Comment on attachment 8880238 [details]
stylo: Fix pseudo element matching for rules in XBL stylesheets (bug 1372876)

https://reviewboard.mozilla.org/r/151598/#review156616

Yup, this is the right fix, thanks for jumping on it TYLin! :)

::: servo/components/style/dom.rs:599
(Diff revision 1)
>      fn xbl_binding_anonymous_content(&self) -> Option<Self::ConcreteNode> {
>          None
>      }
>  
> +    /// Returns the rule hash target given an element.
> +    fn rule_hash_target(&self) -> Self

nit: brace to the same line.

Comment 5

[Emilio Cobos Álvarez [:emilio]]

Are there any reftest for this? if not, we should add one.

Comment 6

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Well, no unexpected pass on try. I think we'll need to add a reftest.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=2a8ab1f0f930ce119694158dd95528f965349d1b

Comment 7

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

I found that non-chrome XBL bindings with stylesheet do not work properly, which blocks me from writing a reftest. I filed a bug 1375513 for that.

Comment 8

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Attachment: Servo PR #17471

Comment 9

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

https://hg.mozilla.org/integration/autoland/rev/4427276b32df