D29542 fixed the bogus checks that was making nested pseudo-elements match
author rules. This adds tests and ends up being just a cleanup, though as it
turns out we it also fixes an issue with ::slotted() matched from
Element.matches.
Differential Revision: https://phabricator.services.mozilla.com/D27529
We always include the combinator for pseudo-elements now (not including it was
just an optimization) in order to not match when nested pseudo-elements are
involved.
We could add a more generic check in `matches_simple_selector` like:
```
if element.is_pseudo_element() {
match *selector {
Component::PseudoElement(..) |
Component::NonTSPseudoClass(..) => {},
_ => return false,
}
}
```
But even that wouldn't be enough to make selectors like `:hover::marker` not
match on the `::before::marker` pseudo-element, plus that code is really hot.
So for now do the check on the `next_element_for_combinator` function. It's a
bit hacky but it's the best I could came up with...
While at it, simplify some checks to use is_pseudo_element() instead of
implemented_pseudo_element() directly.
Only the Rust patch as-is would make markers for ::before and ::after on list
items not show up, so we also need to switch ::marker to use ProbeMarkerStyle()
rather than ProbePseudoElementStyle(), since the marker should exist even if it
matches no rules.
Differential Revision: https://phabricator.services.mozilla.com/D27529
This patch moves some `enum` in `nsIPresShell` which are in public scope into
`mozilla` namespace and change them as `enum class`es.
Unfortunately, only "where to scroll" enum is just defines constants of
percentages of scroll destination. Therefore, this patch makes only them
as `static const`.
Differential Revision: https://phabricator.services.mozilla.com/D28606
This patch changes remaining things under `layout/`. However, there are some
places which still need to use `nsIPresShell`. That will be fixed in a
follow up bug.
Differential Revision: https://phabricator.services.mozilla.com/D27477
`nsPresContext` should use `mozilla::PresShell` directly instead of
`nsIPresShell`. This patch makes it.
Unfortunately, `nsPresContext` and `nsIFrame` have `PresShell()`. Therefore,
we cannot use `PresShell*` in its methods so that this patch uses `mozilla::`
namespace prefix.
It might be better to rename them as `PresShellPtr()` in another bug.
Differential Revision: https://phabricator.services.mozilla.com/D25721
We avoid removing subsumed hints for out-of-flow and column-span frames
in RestyleManager::ProcessPostTraversal(). We should do something
similar here.
Differential Revision: https://phabricator.services.mozilla.com/D24578
Bug 1472637 makes the decision of whether to construct an image frame not depend
on this, so this is sound.
We need to avoid reframing to fix this bug because otherwise we lose track of
the previously painted image.
Differential Revision: https://phabricator.services.mozilla.com/D23127
Since bug 1524480 we set the NS_FRAME_MAY_BE_TRANSFORMED frame bit when needed
in RestyleManager::ProcessRestyledFrames so that it is now redundant to also set
it from KeyframeEffect.
Furthermore, setting frame bits from KeyframeEffect is a little fragile since it
depends on the life cycle of the KeyframeEffect which is independent of the
nsFrame. If we can avoid doing that, we probably should.
Differential Revision: https://phabricator.services.mozilla.com/D21885
For display:table content we generate two frames: a table wrapper frame and an
inner table frame. The styles are applied to the inner frame (referred to as the
style frame), whilst the wrapper frame is the primary frame for the content.
However, in order to make tables with transforms behave as a container for
abspos/fixed-pos content as required by the spec, we apply the transform to the
wrapper frame (bug 722777) by inheriting the transform from inner to wrapper and
then ignoring the transform on the inner frame (bug 722777 and bug 816458).
When handling animations on table elements we need to be careful of this
distinction. in particular, css animations[1] and web animations[2] require that
when we have an unfinished transform animation targetting an element, the
element acts as if it had `will-change: transform` applied and therefore
generates a stacking context. As a result we need to accurately detect when
a frame should be considered as having transform animations applied to it or not
for the purpose of creating a stacking context.
Previously our handling of display:table content was quite inconsistent and
contradictory. For example, `nsIFrame::HasAnimationOfTransform` would check for
a primary frame AND for animations on that frame, despite the fact that we only
ever store animations on the style frame. As a result it could never return true
for either a table wrapper or inner table frame.
This patch attempts to make this handling at least a little more consistent,
producing the following result:
Outer table frame (primary frame):
nsIFrame::IsTransformed → true
nsIFrame::IsCSSTransformed → true
nsIFrame::HasAnimationOfTransform → true
nsLayoutUtils::HasAnimationOfProperty(frame, eCSSProperty_transform) → false
Inner table frame (style frame):
nsIFrame::IsTransformed → false
nsIFrame::IsCSSTransformed → false
nsIFrame::HasAnimationOfTransform → false
nsLayoutUtils::HasAnimationOfProperty(frame, eCSSProperty_transform) → true
We maintain that the NS_FRAME_MAY_BE_TRANSFORMED bit is only set on the primary
frame whilst the mMayHaveTransformAnimation flag is only set on the style frame.
Note that we don't simply always put everything on the primary frame because for
other property types (e.g. opacity) the default setup of putting all styles and
animations on the style frame is simpler and correct. So far it is only
transforms that require special handling to apply the effect to the wrapper
frame.
This patch adds a reftest that fails without the code changes included in this
patch.
[1] https://drafts.csswg.org/css-animations/#animations
[2] https://drafts.csswg.org/web-animations-1/#side-effects-section
Differential Revision: https://phabricator.services.mozilla.com/D21883
Transform display item may have multiple properties, so it's better to
use display item type as the input.
Also, factor some code out of AddAnimationsForProperty, so we can easier
to extend this for multiple properties.
We will pass a list of layers::Animation to the compositor thread. In
this list, the animations belonging to the same property are grouped together,
so we can easily separate the animations by property and sample the animations
for each property on the compositor thread. (Will do this in Bug 1425837.)
Depends on D19628
Differential Revision: https://phabricator.services.mozilla.com/D19629
This doesn't matter yet because all the states that return a change hint are on
stylesheets, but will matter with bug 1472637.
Differential Revision: https://phabricator.services.mozilla.com/D21616
This doesn't matter yet because all the states that return a change hint are on
stylesheets, but will matter with bug 1472637.
Differential Revision: https://phabricator.services.mozilla.com/D21616
Only nsBlockFrame and its subclasses recognize the nsIFrame::eBlockFrame
flag, so we can replace the usage of the flag with either
nsIFrame::IsBlockFrameOrSubclass() or a do_QueryFrame().
Differential Revision: https://phabricator.services.mozilla.com/D20542
This is more consistent with all the other image request code, and handles
pseudo-elements properly without having to add more out-of-band calls to
UpdateStyleOfOwnedChildFrame and such.
Differential Revision: https://phabricator.services.mozilla.com/D20107
This is more consistent with what the Rust bits of the style system do, and
removes a pointer from ComputedStyle which is always nice.
This also aligns the Rust bits with the C++ bits re. not treating xul pseudos as
anonymous boxes. See the comment in nsTreeStyleCache.cpp regarding those.
Can't wait for XUL trees to die.
Depends on D19001
Differential Revision: https://phabricator.services.mozilla.com/D19002
Even less so on reframe, where it's just unsound to do so. I had to give a value
to eSetValue_Internal, since otherwise I cannot check for its presence. I can
further special-case the reframe case if you prefer.
Differential Revision: https://phabricator.services.mozilla.com/D20133
This is more consistent with all the other image request code, and handles
pseudo-elements properly without having to add more out-of-band calls to
UpdateStyleOfOwnedChildFrame and such.
Differential Revision: https://phabricator.services.mozilla.com/D20107
These assertions can happen in certain circumstances (see the referenced bug).
These assertions are not security sensitive, but they affect correctness.
They're old (from before my change), so I prefer dealing with them in a public
bug and stop crashing release for now.
Differential Revision: https://phabricator.services.mozilla.com/D20105
Just for my sanity. I think the other scroll observer is sane after a quick
look, but this will ensure we don't ship security issues.
Differential Revision: https://phabricator.services.mozilla.com/D19725
Typically we set the NS_FRAME_MAY_BE_TRANSFORMED bit on a frame when one of the
following situations arises:
a. We update the keyframes of a KeyframeEffect to include transforms or we
create a new KeyframeEffect that animates transforms (in
KeyframeEffect::SetKeyframes), or
b. We retarget a KeyframeEffect with transforms at a new element (in
KeyframeEffect::SetTarget), or
c. We create an nsFrame with transform animations applied to it (in
nsFrame::Init), or
d. We get a nsChangeHint_AddOrRemoveTransform hint in
RestyleManager::ProcessRestyledFrames and decide to update the frame bit on
the frame and its continuations accordingly.
However, there are some situations where we can have a transform animation on
a frame where none of the above are triggered.
For example, if the style frame is not unavailable (e.g. a display:none
element) when the KeyframeEffect is initialized we will fail to the frame bit in
(a) and if we never retarget the effect we will never set reach (b).
Furthermore, if we have an animation that is "not relevant" (e.g. idle) and
hence not registered with the EffectSet when the frame is initialized we will
fail to set the frame bit in (c).
Finally, if the the animation does not produce a style change that causes the
nsChangeHint_AddOrRemoveTransform bit to be set (e.g. the transform animation
begins at 'none') we will not set the frame bit in (d).
As a result, we can end up failing to set the NS_FRAME_MAY_BE_TRANSFORMED bit
for some content.
The crashtest included in this patch produces such a case and, without the code
changes in this patch, will fail the assertion in ApplyRenderingChangeToTree[1]:
NS_ASSERTION(!(aChange & nsChangeHint_UpdateTransformLayer) ||
aFrame->IsTransformed() ||
aFrame->StyleDisplay()->HasTransformStyle(),
"Unexpected UpdateTransformLayer hint");
That is because although the nsChangeHint_UpdateTransformLayer bit will be set,
aFrame->IsTransformed() will return false because the frame bit has not been
set, and aFrame->StyleDisplay()->HasTransformStyle() will return false because
the animation sets the transform to 'none'.
Not only will this assertion fail, but once we cease flushing style as part of
triggering an animation later in this patch, the reftest
layout/reftests/web-animations/stacking-context-transform-changing-effect.html
will begin to fail. That reftest produces a similar situation to the crashtest
but it currently does not fail because the style flush that happens as part of
creating an animation ensures the style frame is available at the point when the
animation is triggered and hence case (a) from above is hit.
This patch addresses this by detecting the case where we have this change hint
set but not the corresponding frame bit, and setting the frame bit.
[1] https://searchfox.org/mozilla-central/rev/9eb30227b21e0aa40d51d9f9b08bb0b113c5fadb/layout/base/RestyleManager.cpp#1191-1199
Differential Revision: https://phabricator.services.mozilla.com/D18911