Closed Bug 1387906 Opened 7 years ago Closed 7 years ago

stylo: style rules of pseudo-elements may be applied multiple times

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla57
Tracking Status
firefox57 --- fixed

People

(Reporter: xidorn, Unassigned)

References

Details

In devtools/server/tests/mochitest/test_styles-applied.html, the subtest "inheritedSystemStyles" fails because it finds 3 more rules than expected (16 vs. 13) when querying applied rules. After some investigation, the outstanding three rules are: * https://searchfox.org/mozilla-central/rev/b52285fffc13f36eca6b47de735d4e4403b3859e/layout/style/res/ua.css#317-323 * https://searchfox.org/mozilla-central/rev/b52285fffc13f36eca6b47de735d4e4403b3859e/layout/style/res/forms.css#914-928 * https://searchfox.org/mozilla-central/rev/b52285fffc13f36eca6b47de735d4e4403b3859e/layout/style/res/forms.css#938-948 When using Gecko's style system, these three rules show up in the list, but with Stylo, each of them shows up twice, which causes this failure. This may potentially have some performance impact, though probably not a big deal on that aspect.
Are we getting different return values from DOMUtils.getCSSStyleRules in _getElementRules?
Flags: needinfo?(xidorn+moz)
(In reply to Boris Zbarsky [:bz] from comment #1) > Are we getting different return values from DOMUtils.getCSSStyleRules in > _getElementRules? _getElementRules you mean [1]? That seems to be just applying a filter to the result from DOMUtils.getCSSStyleRules. The result of DOMUtils.getCSSStyleRules is from rule nodes [2]. [1] https://searchfox.org/mozilla-central/rev/bd39b6170f04afeefc751a23bb04e18bbd10352b/devtools/server/actors/styles.js#563 [2] https://searchfox.org/mozilla-central/rev/bd39b6170f04afeefc751a23bb04e18bbd10352b/servo/ports/geckolib/glue.rs#1824
Flags: needinfo?(xidorn+moz)
So all those have important and non-important rules, so they generate two rule nodes, one for the normal level, one for the important level. I suspect the API expects them to get filtered out or something?
Oh, that's a good point. We probably should filter them out somehow.
Priority: -- → P3
Ah, good catch. On the Gecko side of DOMUtils.getCSSStyleRules, we have: RefPtr<Declaration> decl = do_QueryObject(ruleNode->GetRule()); if (decl) { The !important rulenodes will return an ImportantStyleData* from GetRule(), which will not QI to Declaration, so nothing will get appended for those rulenodes.
Then what would happen if a rule node only contains important rules?
Gecko always has the non-!important rulenode in the ruletree. The !important bits are added by walking the non-!important branch and asking everything in it whether there are !important rules.
We can just read the declaration block if the level is important, and skip it if it returns true for any_normal. That should also match Gecko's order IIUC. Will write a patch as soon as I get to the office.
(In reply to Emilio Cobos Álvarez [:emilio] from comment #8) > We can just read the declaration block if the level is important, and skip > it if it returns true for any_normal. That should also match Gecko's order > IIUC. Will write a patch as soon as I get to the office. Like so: https://github.com/servo/servo/pull/18008 I just noticed that it won't return the rules in the same order as Gecko for rules only containing important properties, but that doesn't sound like a huge deal I think, and it's pre-existing.
It seems the fix has been merged. I've got unexpected pass for this test with m-c today.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.