Closed Bug 1371112 Opened 4 years ago Closed 4 years ago
stylo: revalidation selectors involving pseudo-elements are broken
In the attached testcase, there should be text above the <hr> but it doesn't appear. The reason for that is that when we go to match the two <div>s against the revalidation seletor [foo]::before they both fail to match, as a result of the changes in bug 1364850. So we end up sharing style, and hence not having a ::before style for the second div. Note that this means the comment I recently added to sharing/mod.rs is wrong, though I think it was right before bug 1364850 landed. This got caught by some random mathml test that does a bunch of ::before/::after stuff when I did a try run with the patch for bug 1329361: we ended up sharing stuff incorrectly because all the revalidation selectors returned "doesn't match".
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
Comment on attachment 8875569 [details] Bug 1371112 tests. https://reviewboard.mozilla.org/r/146984/#review151108 Ouch, good catch Boris. I wonder if it would be better to just ignore pseudo-elements while matching (they're already ignored when inserting in the SelectorMap), that is using `ForStatelessPseudoElement`. Right now we require a pseudo to be there for that, but that isn't necessary anymore (we no longer have that annoying `Selector`/`SelectorInner` distinction, and we can use `selector.has_pseudo_element()` quite easily). In any case, r=me
Attachment #8875569 - Flags: review?(emilio+bugs) → review+
In fairness, this was caught by said mathml test, not me... ;) > that is using `ForStatelessPseudoElement`. Does that do the right thing? Specifically, if we have a selector like "::before:hover" that will return false if we use ForStatelessPseudoElement. I guess that might be ok in practice?
Pushed by email@example.com: https://hg.mozilla.org/integration/autoland/rev/7459489e961d tests. r=emilio
You need to log in before you can comment on or make changes to this bug.