stylo: We don't implement the hack to allow child combinators to match across XBL <children> elements (could we remove it?).

NEW
Unassigned

Status

()

Core
CSS Parsing and Computation
P2
normal
a month ago
25 days ago

People

(Reporter: emilio, Unassigned)

Tracking

(Blocks: 1 bug)

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

a month ago
http://searchfox.org/mozilla-central/rev/7cc377ce3f0a569c5c9b362d589705cd4fecc0ac/layout/style/nsCSSRuleProcessor.cpp#2450

We don't have anything like that right now, which is making my patch at bug 1371130 fail layout/reftests/bugs/232990-1b.xhtml.

Before that patch we were failing to restyle one of the two elements, which happily enough for that test case made it look correct.

I wonder if we can just kill that hack. It'd complicate multiple aspects of the code.
(Reporter)

Comment 1

a month ago
Boris, wdyt, could we remove that hack?
Flags: needinfo?(bzbarsky)
(Reporter)

Comment 2

a month ago
Actually that's not the reason for the failure, but I still think we should remove that hack anyway.

Comment 3

a month ago
The real questions are:

1)  Do we have styles in the Firefox UI that depend on this hack?
2)  Do we have styles in extensions that depend on this hack?

At least when the hack was introduced, the answer to #1 was a resounding "yes".  If I had to guess in the absence of data, I would guess it's still "yes".  But we could try to answer this by the simple expedient of doing a MOZ_CRASH any time the hack causes a match when the non-hacked version would not match and seeing whether try passes (or indeed whether the browser starts up).
Flags: needinfo?(bzbarsky)
(Reporter)

Comment 4

a month ago
(In reply to Boris Zbarsky [:bz] (if a patch has no decent message, automatic r-) from comment #3)
> The real questions are:
> 
> 1)  Do we have styles in the Firefox UI that depend on this hack?
> 2)  Do we have styles in extensions that depend on this hack?
> 
> At least when the hack was introduced, the answer to #1 was a resounding
> "yes".  If I had to guess in the absence of data, I would guess it's still
> "yes".  But we could try to answer this by the simple expedient of doing a
> MOZ_CRASH any time the hack causes a match when the non-hacked version would
> not match and seeing whether try passes (or indeed whether the browser
> starts up).

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

Seems it should really be fine, the only non-intermittent test that fails is the one added in that same patch that tests it (layout/reftests/dom/xbl-children-1.xhtml).

Comment 5

a month ago
Unfortunately, I would expect the failures to be "This part of the browser UI looks wrong", not something we test in CI.  :(  So a clean try run is very much not sufficient to have confidence here...

Comment 6

a month ago
That is, a clean try run with the hack just disabled.  A clean try run with the hack MOZ_CRASHing when it would trigger (as comment 3 suggests) would be much more confidence-inducing.
Priority: -- → P2
You need to log in before you can comment on or make changes to this bug.