The key here is that we only filter longhands if the shorthand is accessible to
content and vice-versa. This prevents the bug that prevented me to land this
patch before, which was us not expanding properly chrome-only shorthands.
Again, this is incomplete, and I need to teach LonghandsToSerialize to get a
potentially incomplete list of properties, and all that.
I looked at what were we doing in that loop, and we're doing tons of dumb stuff.
In particular, we try to serialize the "all" shorthand all the time. This patch
prevents us from trying to serialize shorthands that we've already tried to
serialize.
By making AnimationValue have the same representation as PropertyDeclaration
and Void variants for non-animatable properties, we know by constructions
that all properties have the same discriminant in both.
We further partition the variants to retrieve all properties that are
defined as simple keywords, and we clone them by copying the value
as a smaller enum made of only the keyword variants.
We first check that the discriminants are equal, then we pattern match only
on `*self` and use `PropertyDeclarationVariantRepr` to look into `other` directly.
This is in preparation of a cascade optimization for custom properties.
This fixes various fishiness around our StyleBuilder stuff. In particular,
StyleBuilder::for_derived_style (renamed to for_animation) is only used to
compute specified values, and thus doesn't need to know about rules, visited
style, or other things like that.
The flag propagation that was done in StyleAdjuster is now done in StyleBuilder,
since we know beforehand which ones are always inherited, and it simplified the
callers and the StyleAdjuster code. It also fixed some fishiness wrt which flags
were propagated to anon boxes and text.
The text-decoration-lines bit is interesting, because the way it was implemented
in #17722 meant that display: contents elements did get HAS_DECORATION_LINES
flags only if its parent also had it, so in practice the Contents check
preserves behavior, but it's only an optimization looking at Gecko's call-sites,
so we can remove it too.
MozReview-Commit-ID: 6BHCyEO2U8c
Now that we have an Element around on cascade, we can stop using the cascade
flags mechanism to pass various element-related state, like "is this element the
root", or "should it use the item-based display fixup".
That fixes handwaviness in the handling of those flags from style reparenting,
and code duplication to handle tricky stuff like :visited.
There are a number of other changes that are worth noticing:
* skip_root_and_item_based_display_fixup is renamed to skip_item_display_fixup:
TElement::is_root() already implies being the document element, which by
definition is not native anonymous and not a pseudo-element.
Thus, you never get fixed-up if your NAC or a pseudo, which is what the code
tried to avoid, so the only fixup with a point is the item one, which is
necessary.
* The pseudo-element probing code was refactored to return early a
Option::<CascadeInputs>::None, which is nicer than what it was doing.
* The visited_links_enabled check has moved to selector-matching time. The rest
of the checks aren't based on whether the element is a link, or are properly
guarded by parent_style.visited_style().is_some() or visited_rules.is_some().
Thus you can transitively infer that no element will end up with a :visited
style, not even from style reparenting.
Anyway, the underlying reason why I want the element in StyleAdjuster is because
we're going to implement an adjustment in there depending on the tag of the
element (converting display: contents to display: none depending on the tag), so
computing that information eagerly, including a hash lookup, wouldn't be nice.
This more concrete wrapper type can write a prefix the very first time something
is written to it. This allows removing plenty of useless monomorphisations caused
by the former W/SequenceWriter<W> pair of types.
Not super-proud of this one, but it's the easiest way I could think of.
The changeset looks bigger than what it is, because while at it I've rewrapped a
fair amount of functions around to use proper block indentation.
Alternatives are parameterizing Stylist by <E>, which is not fun, or moving the
concrete element from layout_thread to layout, but that implies layout depending
on script, which isn't fun either.
Other alternative is implementing an empty enum and making anon boxes work on
it. It has the advantage of removing the annoying type parameter, but the
disadvantage of instantiating `cascade` twice, which isn't great, and having to
maintain all the boilerplate of a `TElement` implementation that just does
nothing.
That way color pickers work even when this setting is on. We always know that
pseudo-element style attributes are trusted.
Bug: 1429248
Reviewed-by: heycam
MozReview-Commit-ID: 6IdmRaMzj6I
It is bogus, because it depends on the display property as it's cascaded, but
the display property can change afterwards, for example, if we get blockified
because we're the root element or a flex item.
Replace it with a normal field instead.
Also, it carries some weight, because it's the last property that uses this
concept of "derived" property, and "custom cascade". So we can remove some code
after this.
Compute it after the cascade process in StyleAdjuster.