555627 - CSS Transition Cannot Be Applied to :before/:after insertions

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Blair Mitchelmore]

User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.3a4pre) Gecko/20100327 Minefield/3.7a4pre
Build Identifier: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.3a4pre) Gecko/20100327 Minefield/3.7a4pre

content inserted with :after/:before CSS declarations will transition according to its parent element provided you don't override that style in the css for the inserted content, but you cannot apply individual transitions to the :before/:after blocks themselves.

See the linked demo for a visual example.

Reproducible: Always

Steps to Reproduce:
1. Apply a transition to a :before/:after declaration 
2. Activate the transition
Actual Results:  
The style switches immediately.

Expected Results:  
The style should transition according to the -moz-transition-* values set.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: Slightly simpler testcase just showing the problem

Comment 2

[Boris Zbarsky [:bzbarsky]]

This is happening because we're not consistent about the aElement we pass to GetElementTransitions.  From nsTransitionManager::StyleContextChanged we pass in the element we synthesize for the :before or :after.  From WalkTransitionRule we're passing in the real element the pseudo is a pseudo-element for.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Attachment: Fix

That patch fixes the problem, but I haven't sorted through writing a test for this.  David, is there a short painless way to hook into the existing tests here, or do I need to do more work than that?

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 435775 [details] [diff] [review]
Fix

Could you:
 * document in nsTransitionManager.h that StyleContextChanged expects aElement to be the generated before/after element for style contexts with :before and :after pseudos.

Could you also add an assertion in GetElementTransitions that it is never passed an element whose tag is mozgeneratedcontenttbefore/mozgeneratedcontentafter?

As for tests, it's relatively straightforward to write a "stupid" test in a standalone manner by either (a) using a large transition-delay and checking that nothing has changed immediately after triggering the change or (b) as test_transitions_per_property does, using a transition-delay of -1/4 of the duration, and checking that you get the correct interpolation right after triggering the change (I'd suggest not using -1/2 because it tests less, though I haven't converted ..._per_property away from it)

r=dbaron with that

Comment 5

[Boris Zbarsky [:bzbarsky]]

> document in nsTransitionManager.h that StyleContextChanged expects

Done.

> Could you also add an assertion in GetElementTransitions that it is never
> passed an element whose tag is
> mozgeneratedcontenttbefore/mozgeneratedcontentafter?

I don't think I can; there's nothing that prevents web pages from having elements with those localnames.

I wrote a test for this in test_transitions.html, but it looks like the patch no longer works for some reason (which is too bad; had I done the test earlier, whatever broke it would have made the tree red!).  I'm working on bisecting now to find out when it stopped working.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Oh, and my test is not working even in older builds, because computed style on ::before/::after doesn't see the transitioning values (because it always resolves instead of using the frame).  I need to adjust it.  But even the testcase in this bug is broken in current builds with this patch: it keeps showing the red color.

Comment 7

[Boris Zbarsky [:bzbarsky]]

And the reason for that is that RestyleForAnimation is called on the parent element for ::before and ::after, and we now only do "self" restyles in RestyleForAnimation.

Comment 8

[Boris Zbarsky [:bzbarsky]]

But shouldn't computed style work?  Does nsTransitionManager::WalkTransitionRule need to check IsProcessingRestyles() before deciding to not walk the rule?

Comment 9

[Boris Zbarsky [:bzbarsky]]

David, see comment 8?

Comment 10

[Boris Zbarsky [:bzbarsky]]

Attachment: Fix that works on trunk, with test

Comment 11

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 453112 [details] [diff] [review]
Fix that works on trunk, with test

r=dbaron

And I agree with the change suggested in comment 8 (as I think we discussed on IRC).

Comment 12

[Boris Zbarsky [:bzbarsky]]

Attachment: Interdiff showing the changes I made for comment 8

Comment 13

[Boris Zbarsky [:bzbarsky]]

Pushed http://hg.mozilla.org/mozilla-central/rev/6ce0346e2560