(Path is actually r=froydnj.)
Bug 1400459 devirtualized nsIAtom so that it is no longer a subclass of
nsISupports. This means that nsAtom is now a better name for it than nsIAtom.
MozReview-Commit-ID: 91U22X2NydP
We don't follow this bit intentionally because we know that even if it's set,
when none of the other two bits are set there are no other restyle / change
hints down the tree.
We rely on the frame constructor to clean the mess up, though, and it doesn't
really do a good work about it.
In particular, the case we're hitting on the test-case is:
<body descendant-need-frames change=reconstruct style="display: table-column-group">
<div descendant-need-frames>
<div descendant-need-frames>
<span needs-frame></span>
</div>
</div>
</body>
When we see we need to reconstruct the body, we call
ClearRestyleStateFromSubtree, but that doesn't do much now, since we don't
follow the descendant-need-frames bits.
Then, when we reconstruct the content, we arrive at[1] when constructing the
first child <div>. The <div> flags have been cleared, but not the children's!
Then a text-node is inserted in a <div>, breaking all sorts of invariants.
This is the easiest fix. Other fixes include clearing the flags on
SetAsUndisplayedContent. But that implies not clearing them in
ShouldCreateItemsForChild, and doing that somewhere more sensible.
I suspect it's not too hard, but that's a slightly more risky change, will do it
if you prefer it.
[1]: http://searchfox.org/mozilla-central/rev/3dbb47302e114219c53e99ebaf50c5cb727358ab/layout/base/nsCSSFrameConstructor.cpp#6092
MozReview-Commit-ID: 7026wkQLQkz
We don't follow this bit intentionally because we know that even if it's set,
when none of the other two bits are set there are no other restyle / change
hints down the tree.
We rely on the frame constructor to clean the mess up, though, and it doesn't
really do a good work about it.
In particular, the case we're hitting on the test-case is:
<body descendant-need-frames change=reconstruct style="display: table-column-group">
<div descendant-need-frames>
<div descendant-need-frames>
<span needs-frame></span>
</div>
</div>
</body>
When we see we need to reconstruct the body, we call
ClearRestyleStateFromSubtree, but that doesn't do much now, since we don't
follow the descendant-need-frames bits.
Then, when we reconstruct the content, we arrive at[1] when constructing the
first child <div>. The <div> flags have been cleared, but not the children's!
Then a text-node is inserted in a <div>, breaking all sorts of invariants.
This is the easiest fix. Other fixes include clearing the flags on
SetAsUndisplayedContent. But that implies not clearing them in
ShouldCreateItemsForChild, and doing that somewhere more sensible.
I suspect it's not too hard, but that's a slightly more risky change, will do it
if you prefer it.
[1]: http://searchfox.org/mozilla-central/rev/3dbb47302e114219c53e99ebaf50c5cb727358ab/layout/base/nsCSSFrameConstructor.cpp#6092
MozReview-Commit-ID: 7026wkQLQkz
Unlike GeckoRestyleManager::UpdateOnlyAnimationStyles which is called from
*both* GeckoRestyleManager::ProcessPendingRestyles and FlushThrottledStyles,
ServoRestyleManager::UpdateOnlyAnimationStyles is only called from
FlushThrottledStyles which is what we use to update transform animations before
doing hit-testing. In this case, there doesn't appear to be any situation where
throttled SMIL animations need to be updated since SMIL animations are only
throttled when they apply to display:none content (bug 1209405 and bug 1322970)
in which case they should not affect hit testing.
MozReview-Commit-ID: 4bt7hCSa7Xr
This was added in bug 1384769 for passing into Servo_TakeChangeHint,
but bug 1388031 changed Servo_TakeChangeHint to no longer take it, and
there is nothing else in the function which uses the flags.
MozReview-Commit-ID: LvZkJZHENUB
In particular, this avoids the stupid calls in display: none roots.
The cache stuff should be unnecessary, but there's still some fishy stuff going
on.
MozReview-Commit-ID: LgnW0k1gmsN
Bindgen bitfield enums don't work as return values with the Linux 32-bit ABI at
the moment because they wrap the value in a struct.
This causes the Rust side to believe the caller will pass along space for the
struct return value, while C++ believes it's just an integer value.
MozReview-Commit-ID: 6qqVVfU8Mb2
This patch changes UpdateAnimationOnlyStyles to only flush animation styles if
there are throttled animation styles that could affect hit-testing and renames
the function to UpdateAnimationStylesForHitTesting at the same time.
For GeckoRestyleManager, the original UpdateAnimationOnlyStyles which flushes
animation styles if there are any pending animation styles, is renamed to
UpdateAnimationStyles for consistency.
MozReview-Commit-ID: 89UleXjI2OE
I'm still not quite sure how may we end up restyling a text node under there,
but all my attempts to build a test-case have failed.
Anyway this is the right thing to do.
MozReview-Commit-ID: FitqSKhNt2n
Now that we do process normal traversal even in the case of throttled animation
flush so that we don't need to do special handling for the case.
Note about the comment in has_current_styles():
the remaining animation hints is not caused by either this patch or the
previous patch in this patch series, it's been there in the first place, but
it should be fixed somehow later. See bug 1389675.
MozReview-Commit-ID: JojHufxNCiS
This fixes the testcase in the bug, which removes and reinserts
some elements. Our invariants require us not to set the dirty
descendants bits on unstyled elements.
MozReview-Commit-ID: 1eESZjNSURG
We have to set mHaveNonAnimationRestyles if we have attributes changed
(note: we increase the animation generation only if mHaveNonAnimationRestyles
is set). Attributes changed may create a new transition, and we use the
animation generation as the order of the transition, so
Element::GetAnimations() can return transitions with correct order.
Besides, I think ContentStateChanged() will not trigger a new
transition, so we don't need to make mHaveNonAnimationRestyles there.
MozReview-Commit-ID: J5XgW8nqeLH
This doesn't actually implement style context reparenting in the style set yet; that part is next.
There is one behavior difference being introduced here compared to Gecko: we
don't reparent the first block piece of an {ib} (block-inside-inline) split
whose first inline piece is being reparented. This is actually a correctness
fix. In this testcase:
<style>
#target { color: green; }
#target::first-line { color: red; }
</style>
<div id="target">
<span>
<div>This should be green</div>
</span>
</div>
Gecko makes the text red, while every other browser makes it green.
We're preserving Gecko's behavior for out-of-flows in first-line so far, but
arguably it's wrong per spec and doesn't match other browsers either. We can
look into changing it later.
MozReview-Commit-ID: 5eC6G449Mlh
