Last Comment Bug 625289 - CSS transitions don't start due to frame reconstruction of ancestor or self that's "simultaneous" with the change that would start the transition
: CSS transitions don't start due to frame reconstruction of ancestor or self t...
Status: VERIFIED FIXED
: dev-doc-complete, testcase
Product: Core
Classification: Components
Component: CSS Parsing and Computation (show other bugs)
: unspecified
: All All
-- normal with 22 votes (vote)
: mozilla34
Assigned To: David Baron :dbaron: ⌚️UTC-8
:
: Jet Villegas (:jet)
Mentors:
: 718000 764012 788677 800927 821976 896943 915168 923538 944001 946573 1011153 1031007 (view as bug list)
Depends on:
Blocks: 764012 788677 800927 821976 915168 1011153 1248402
  Show dependency treegraph
 
Reported: 2011-01-12 21:03 PST by Rob Arnold [:robarnold]
Modified: 2016-11-11 02:00 PST (History)
62 users (show)
dbaron: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
-
34+


Attachments
Testcase demonstating the problem (2.76 KB, text/html)
2011-01-12 21:03 PST, Rob Arnold [:robarnold]
no flags Details
patch 1 - Add comment about dependence on synchronous frame reconstruction (1.50 KB, patch)
2014-07-31 17:41 PDT, David Baron :dbaron: ⌚️UTC-8
cam: review+
Details | Diff | Splinter Review
patch 2 - Create wrapper function around the only calling pattern of ComputeStyleChangeFor (5.20 KB, patch)
2014-07-31 17:41 PDT, David Baron :dbaron: ⌚️UTC-8
cam: review+
Details | Diff | Splinter Review
patch 3 - Add types for storing the style contexts of elements currently being reframed (2.23 KB, patch)
2014-07-31 17:41 PDT, David Baron :dbaron: ⌚️UTC-8
cam: review+
Details | Diff | Splinter Review
patch 4 - Add member variable to restyle manager for currently reframing style contexts (3.26 KB, patch)
2014-07-31 17:41 PDT, David Baron :dbaron: ⌚️UTC-8
cam: review+
Details | Diff | Splinter Review
patch 5 - Create a ReframingStyleContexts struct during restyling (1.46 KB, patch)
2014-07-31 17:41 PDT, David Baron :dbaron: ⌚️UTC-8
cam: review+
Details | Diff | Splinter Review
patch 6 - Store style contexts being reframed in the ReframingStyleContexts struct (2.40 KB, patch)
2014-07-31 17:41 PDT, David Baron :dbaron: ⌚️UTC-8
cam: review+
Details | Diff | Splinter Review
patch 7 - Expose TryStartingTransition (3.66 KB, patch)
2014-07-31 17:41 PDT, David Baron :dbaron: ⌚️UTC-8
cam: review+
Details | Diff | Splinter Review
patch 8 - Add FIXME comments suggesting additional use of ResolveStyleContext when resolving the root frame (2.24 KB, patch)
2014-07-31 17:42 PDT, David Baron :dbaron: ⌚️UTC-8
cam: review+
Details | Diff | Splinter Review
patch 9 - Convert nsCSSFrameConstructor::ResolveStyleContext away from early returns (2.49 KB, patch)
2014-07-31 17:42 PDT, David Baron :dbaron: ⌚️UTC-8
cam: review+
Details | Diff | Splinter Review
patch 10 - Check for difference in HasPseudoElementData before starting transitions (3.24 KB, patch)
2014-07-31 17:42 PDT, David Baron :dbaron: ⌚️UTC-8
cam: review+
Details | Diff | Splinter Review
patch 11 - Call RestyleManager::TryStartingTransition during frame construction (4.00 KB, patch)
2014-07-31 17:42 PDT, David Baron :dbaron: ⌚️UTC-8
cam: review+
Details | Diff | Splinter Review
patch 12 - Tests for transitions on elements that are reframing (6.11 KB, patch)
2014-07-31 17:42 PDT, David Baron :dbaron: ⌚️UTC-8
cam: review+
Details | Diff | Splinter Review

Description User image Rob Arnold [:robarnold] 2011-01-12 21:03:14 PST
Created attachment 503405 [details]
Testcase demonstating the problem

See attached testcase. Transitions don't appear to skip in Chrome 10 but do sometimes in Firefox 4.0b10pre.
Comment 1 User image David Baron :dbaron: ⌚️UTC-8 2011-01-13 11:15:49 PST
I suspect this is likely a bug in interpolation of transforms (where some cases fail to interpolate), but I haven't debugged it yet.
Comment 2 User image David Baron :dbaron: ⌚️UTC-8 2011-01-16 10:26:12 PST
Er, no, I think it's a bug on simultaneously starting transforms on elements nested inside each other.
Comment 3 User image David Baron :dbaron: ⌚️UTC-8 2011-01-16 12:28:25 PST
http://hg.mozilla.org/users/dbaron_mozilla.com/patches/raw-file/482b08600e0a/correct-covering fixes it, although I don't completely understand why yet.  (I'm pretty confident in the patch; I'm just not sure why it's this important for a non-inherited property.)
Comment 4 User image David Baron :dbaron: ⌚️UTC-8 2011-01-16 12:39:19 PST
Actually, I'm not confident in the patch; I think it's wrong.
Comment 5 User image David Baron :dbaron: ⌚️UTC-8 2011-01-16 12:44:06 PST
I guess the problem is the "if we're reframing, let the frame keep its old style context and don't reresolve the kids" code in nsFrameManager::ReResolveStyleContext.
Comment 6 User image David Baron :dbaron: ⌚️UTC-8 2011-01-16 12:50:09 PST
I'm not sure it's realistic to fix this for Firefox 4.  The basic problem is that switching between "no transform" and "have a transform" requires a reframe, since it changes some invariants of the frame tree structure, like where fixed-positioned frames go.  (I wish it didn't, but that's what the spec says.)

So if you're going to be dynamically switching between no-transform and scale(2), you're a lot better off switching between no-transform expressed as the relevant identity, e.g., scale(1) and scale(2), translateX(0px) and translateX(100px), etc., rather than using 'none'.
Comment 7 User image David Baron :dbaron: ⌚️UTC-8 2012-06-13 08:46:39 PDT
*** Bug 764012 has been marked as a duplicate of this bug. ***
Comment 8 User image Daniel Trebbien 2013-03-03 08:48:21 PST
Is this still an issue?  I am trying the test case on Mac OS 10.7.5, but I do not see any difference in Firefox 19, Safari 6.0.2, or Chrome 25.0.1364.152.
Comment 9 User image Rob Arnold [:robarnold] 2013-03-04 09:55:06 PST
I wasn't able to reproduce it on a recent build though my machine (and probably Firefox) is much faster now than it was when I reported the bug two years ago.
Comment 10 User image Jakub Chodorowicz 2013-06-20 14:43:45 PDT
Yep, it's still prsent in v. 21
example: http://jsfiddle.net/chodorowicz/bc2YC/5/
Comment 11 User image Sergei Vasiliev 2013-07-10 01:57:17 PDT
It's still in v. 22 either
http://jsfiddle.net/jCfwc/
Comment 12 User image ritterb82 2013-08-27 12:18:04 PDT
I just want to confirm this is an issue ( see the http://jsfiddle.net/jCfwc/ above ) in FF 23.0.1 MAC.
Comment 13 User image ritterb82 2013-08-27 18:47:48 PDT
Also, I think this should be a relatively "higher" priority issue because this isn't just about the animation visually not working ( which i admit might not be a big deal ).  But instead, when this happens the animation event "transitionend" doesn't fire when the animation is skipped.

So actually, writing any sort of logic with the events fail, which I think warrants an higher priority. Its NOT just a presentation thing.
Comment 14 User image Stephanie Hobson [:shobson] 2013-09-10 12:13:00 PDT
Would changing overflow cause a frame reconstruction? I maybe have just encountered this bug using overflow instead of position. In webkit the menu tray slides out from the left as expected: http://jsfiddle.net/xGZcz/
Comment 15 User image Boris Zbarsky [:bz] (still a bit busy) 2013-09-10 12:14:52 PDT
> Would changing overflow cause a frame reconstruction?

Yes.
Comment 16 User image David Baron :dbaron: ⌚️UTC-8 2013-09-19 14:46:13 PDT
*** Bug 718000 has been marked as a duplicate of this bug. ***
Comment 17 User image David Baron :dbaron: ⌚️UTC-8 2013-09-19 14:49:09 PDT
*** Bug 896943 has been marked as a duplicate of this bug. ***
Comment 18 User image Boris Zbarsky [:bz] (still a bit busy) 2013-10-03 21:33:06 PDT
*** Bug 923538 has been marked as a duplicate of this bug. ***
Comment 19 User image Binyamin 2013-10-04 04:19:44 PDT
Test-case http://jsbin.com/eZOsEja/3/edit from #923538.
Comment 20 User image ritterb82 2013-10-17 11:13:39 PDT Comment hidden (advocacy)
Comment 21 User image Peter Rottmann 2013-11-20 08:45:17 PST
Problem still exists in 25.0.1.

Here is a simple example with the problem:
http://jsfiddle.net/FJJF6/1/

1. outer div changed to position fixed
and the same time
2. yellow box should fade in ... but it did not fade.
Comment 22 User image Boris Zbarsky [:bz] (still a bit busy) 2013-11-27 13:20:30 PST
*** Bug 944001 has been marked as a duplicate of this bug. ***
Comment 23 User image ritterb82 2013-12-04 09:27:07 PST Comment hidden (advocacy)
Comment 24 User image ritterb82 2013-12-04 09:27:50 PST Comment hidden (advocacy)
Comment 25 User image Boris Zbarsky [:bz] (still a bit busy) 2013-12-05 07:12:32 PST
*** Bug 946573 has been marked as a duplicate of this bug. ***
Comment 26 User image Louis Acresti 2013-12-05 11:07:41 PST
Wasted hours tracking this one down, yesterday.

Here's yet another test case: http://jsbin.com/UwaXOBix/3/edit
Comment 27 User image ritterb82 2014-02-13 14:04:48 PST Comment hidden (advocacy)
Comment 28 User image slmgc 2014-02-15 07:37:12 PST Comment hidden (advocacy)
Comment 29 User image duesentrieb.daniel 2014-03-13 10:26:36 PDT
Confirmed, another test case: http://codepen.io/anon/pen/Dglmi
Comment 30 User image ritterb82 2014-04-03 07:49:43 PDT Comment hidden (advocacy)
Comment 31 User image Daniel Schwiperich 2014-04-11 04:05:48 PDT
Stumbling more and more often over this quite annoying bug in recent projects. 
Here is another really small test case
http://jsfiddle.net/dPg4w/1/
Comment 32 User image Robert 2014-04-12 08:42:19 PDT
Here's another use case (functionality) that doesn't work in Firefox (sad face). http://jsfiddle.net/langbert/mDcQd/ I really hope this bug can move along. It's making transition so very problematic.
Comment 33 User image Robert 2014-04-12 08:46:26 PDT
By the way, here's my hackish workaround: http://jsfiddle.net/langbert/mDcQd/2/ Note that in the previous version, there was no transitionend event firing, rendering the whole screen unclickable.
Comment 34 User image ritterb82 2014-04-16 09:18:44 PDT Comment hidden (advocacy)
Comment 35 User image Angelina Fabbro (:angelina) 2014-04-16 10:27:32 PDT
Bumping this - affects the ability of developers working with Firefox OS to create compelling user interactions. 

Two designer-developers have asked me personally about this bug this week. That.. is pretty unusual for any bug, so I tend to take that as a sign that there's a sincere need beyond my own observations.

Cheers & thanks in advance with much gratitude on this one.
Comment 36 User image David Baron :dbaron: ⌚️UTC-8 2014-04-16 11:27:32 PDT
This is on my list of things that are important to work on.
Comment 37 User image Boris Zbarsky [:bz] (still a bit busy) 2014-06-26 23:19:43 PDT
*** Bug 1031007 has been marked as a duplicate of this bug. ***
Comment 38 User image nicmarson 2014-07-01 15:26:16 PDT
I updated Robert's fiddle with a solution:
http://jsfiddle.net/mDcQd/8/

The key is to have position defined on the container with overflow: hidden.

He had it set once the class was added like so:

.siteNavOpen body {
    overflow: hidden;
}

instead, body needs to have it set beforehand

body {
    overflow: hidden;
    position: relative;
}
Comment 39 User image nicmarson 2014-07-02 09:10:43 PDT
However, this prevents the window scrollbar, so I use a JavaScript function to apply overflow: hidden just before the transitions are triggered.

$('body').addClass('menu__open');
setTimeout(function(){
    $(menu).addClass('menu--open');
}, 25);
Comment 40 User image ritterb82 2014-07-15 08:28:14 PDT Comment hidden (advocacy)
Comment 41 User image Rachel Nabors 2014-07-15 11:16:18 PDT Comment hidden (advocacy)
Comment 42 User image cepedagomez 2014-07-24 00:18:21 PDT Comment hidden (advocacy)
Comment 43 User image Thierry 2014-07-30 13:47:55 PDT Comment hidden (off-topic)
Comment 44 User image David Baron :dbaron: ⌚️UTC-8 2014-07-30 17:11:40 PDT
https://tbpl.mozilla.org/?tree=Try&rev=cf851a097840
Comment 45 User image David Baron :dbaron: ⌚️UTC-8 2014-07-30 18:56:43 PDT
https://tbpl.mozilla.org/?tree=Try&rev=28273e2a0b86
(missed a patch that these depended on)
Comment 46 User image David Baron :dbaron: ⌚️UTC-8 2014-07-31 17:41:34 PDT
Created attachment 8465883 [details] [diff] [review]
patch 1 - Add comment about dependence on synchronous frame reconstruction
Comment 47 User image David Baron :dbaron: ⌚️UTC-8 2014-07-31 17:41:37 PDT
Created attachment 8465884 [details] [diff] [review]
patch 2 - Create wrapper function around the only calling pattern of ComputeStyleChangeFor
Comment 48 User image David Baron :dbaron: ⌚️UTC-8 2014-07-31 17:41:41 PDT
Created attachment 8465885 [details] [diff] [review]
patch 3 - Add types for storing the style contexts of elements currently being reframed
Comment 49 User image David Baron :dbaron: ⌚️UTC-8 2014-07-31 17:41:45 PDT
Created attachment 8465886 [details] [diff] [review]
patch 4 - Add member variable to restyle manager for currently reframing style contexts
Comment 50 User image David Baron :dbaron: ⌚️UTC-8 2014-07-31 17:41:49 PDT
Created attachment 8465887 [details] [diff] [review]
patch 5 - Create a ReframingStyleContexts struct during restyling
Comment 51 User image David Baron :dbaron: ⌚️UTC-8 2014-07-31 17:41:54 PDT
Created attachment 8465888 [details] [diff] [review]
patch 6 - Store style contexts being reframed in the ReframingStyleContexts struct
Comment 52 User image David Baron :dbaron: ⌚️UTC-8 2014-07-31 17:41:58 PDT
Created attachment 8465889 [details] [diff] [review]
patch 7 - Expose TryStartingTransition
Comment 53 User image David Baron :dbaron: ⌚️UTC-8 2014-07-31 17:42:02 PDT
Created attachment 8465890 [details] [diff] [review]
patch 8 - Add FIXME comments suggesting additional use of ResolveStyleContext when resolving the root frame

This isn't actually a problem for this patch series because the root
element can't have an ancestor that's reframed because it has no
ancestors, and reframes of the element itself trigger a restyling
operation that does actually start transitions.
Comment 54 User image David Baron :dbaron: ⌚️UTC-8 2014-07-31 17:42:06 PDT
Created attachment 8465891 [details] [diff] [review]
patch 9 - Convert nsCSSFrameConstructor::ResolveStyleContext away from early returns
Comment 55 User image David Baron :dbaron: ⌚️UTC-8 2014-07-31 17:42:10 PDT
Created attachment 8465892 [details] [diff] [review]
patch 10 - Check for difference in HasPseudoElementData before starting transitions
Comment 56 User image David Baron :dbaron: ⌚️UTC-8 2014-07-31 17:42:15 PDT
Created attachment 8465893 [details] [diff] [review]
patch 11 - Call RestyleManager::TryStartingTransition during frame construction
Comment 57 User image David Baron :dbaron: ⌚️UTC-8 2014-07-31 17:42:18 PDT
Created attachment 8465894 [details] [diff] [review]
patch 12 - Tests for transitions on elements that are reframing

Without the patch, most of the tests fail (although the e1 test and the
two root tests pass).  With the patch, all tests pass.

I presume the three tests that pass prior to the patch pass because the
restyle actually occurs and starts the transition (which is stored on
the element) before the style context is destroyed.  (The same should, I
think, happen for a style context coming out of the undisplayed map.)
Comment 58 User image Cameron McCormack (:heycam) 2014-07-31 21:08:20 PDT
Comment on attachment 8465884 [details] [diff] [review]
patch 2 - Create wrapper function around the only calling pattern of ComputeStyleChangeFor

Review of attachment 8465884 [details] [diff] [review]:
-----------------------------------------------------------------

::: layout/base/RestyleManager.h
@@ +100,5 @@
> +  void ComputeAndProcessStyleChange(nsIFrame* aFrame,
> +                                    nsChangeHint aMinChange,
> +                                    RestyleTracker& aRestyleTracker,
> +                                    nsRestyleHint aRestyleHint);
> +

Let's make this and ComputeStyleChangeFor private, since they're not used outside RestyleManager.
Comment 59 User image Cameron McCormack (:heycam) 2014-07-31 21:19:49 PDT
Comment on attachment 8465888 [details] [diff] [review]
patch 6 - Store style contexts being reframed in the ReframingStyleContexts struct

Review of attachment 8465888 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with these three things.

::: layout/generic/nsFrame.cpp
@@ +651,5 @@
>      }
>    }
>  
>    // This needs to happen before shell->NotifyDestroyingFrame because that
>    // clears our Properties() table.

Should this comment be moved to just above the TransferActivityToContent call?  I think that's what it's referring to.

@@ +653,5 @@
>  
>    // This needs to happen before shell->NotifyDestroyingFrame because that
>    // clears our Properties() table.
>    bool isPrimaryFrame = (mContent && mContent->GetPrimaryFrame() == this);
> +  // FIXME: Check that this works for ::before/::after.

Can you remove this FIXME given the tests in the final patch?

@@ +658,5 @@
>    if (isPrimaryFrame) {
>      ActiveLayerTracker::TransferActivityToContent(this, mContent);
> +
> +    // Unfortunately, we need to do this for all frames being reframed
> +    // and not only those whose currently style involves CSS

s/currently/current/
Comment 60 User image Cameron McCormack (:heycam) 2014-07-31 21:21:15 PDT
Comment on attachment 8465889 [details] [diff] [review]
patch 7 - Expose TryStartingTransition

Review of attachment 8465889 [details] [diff] [review]:
-----------------------------------------------------------------

::: layout/base/RestyleManager.h
@@ +166,5 @@
> +   */
> +  static void
> +  TryStartingTransition(nsPresContext *aPresContext, nsIContent *aContent,
> +                        nsStyleContext *aOldStyleContext,
> +                        nsRefPtr<nsStyleContext> *aNewStyleContext /* inout */);

Please put the "*"s next to the types, and in the .cpp file too.
Comment 61 User image Cameron McCormack (:heycam) 2014-07-31 21:26:01 PDT
Comment on attachment 8465890 [details] [diff] [review]
patch 8 - Add FIXME comments suggesting additional use of ResolveStyleContext when resolving the root frame

Review of attachment 8465890 [details] [diff] [review]:
-----------------------------------------------------------------

Is the issue then that the new style context might not have the right parent (as that seems to be what nsCSSFrameConstructor::ResolveStyleContext does)?
Comment 62 User image Cameron McCormack (:heycam) 2014-07-31 21:30:32 PDT
Comment on attachment 8465892 [details] [diff] [review]
patch 10 - Check for difference in HasPseudoElementData before starting transitions

Review of attachment 8465892 [details] [diff] [review]:
-----------------------------------------------------------------

::: layout/style/nsTransitionManager.cpp
@@ +146,5 @@
> +    // bail.  We can't hit this codepath for normal style changes
> +    // involving moving frames around the boundaries of these
> +    // pseudo-elements since we don't call StyleContextChanged from
> +    // ReparentStyleContext.  However, we can hit this codepath during
> +    // the handling of transitions that start across reframes.

Can you give me a short example demonstrating when we do get in here?  If it's short enough maybe stick it in the comment here.
Comment 63 User image Cameron McCormack (:heycam) 2014-07-31 21:39:19 PDT
Comment on attachment 8465893 [details] [diff] [review]
patch 11 - Call RestyleManager::TryStartingTransition during frame construction

Review of attachment 8465893 [details] [diff] [review]:
-----------------------------------------------------------------

::: layout/base/nsCSSFrameConstructor.cpp
@@ +1821,5 @@
> +    RestyleManager()->GetReframingStyleContexts();
> +  if (rsc) {
> +    nsStyleContext* oldStyleContext =
> +      (isBefore ? rsc->mBeforePseudoContexts
> +                : rsc->mAfterPseudoContexts).GetWeak(aParentContent);

May be slightly more readable with something like:

  nsStyleContext* oldStyleContext = rsc->Get(aParentContent, aPseudoElement);

and an appropriate definition of ReframingStyleContexts::Get that looks up the right table and asserts the pseudo is expected.  Up to you.

@@ +4783,5 @@
>  
> +  RestyleManager::ReframingStyleContexts* rsc =
> +    RestyleManager()->GetReframingStyleContexts();
> +  if (rsc) {
> +    nsStyleContext* oldStyleContext = rsc->mElementContexts.GetWeak(aContent);

And similarly here with a single argument version of Get().
Comment 64 User image Cameron McCormack (:heycam) 2014-07-31 21:41:45 PDT
Comment on attachment 8465888 [details] [diff] [review]
patch 6 - Store style contexts being reframed in the ReframingStyleContexts struct

If you add the ReframingStyleContexts::Get method in patch 11, you should use a Put method rather than switching on the pseudo type here.
Comment 65 User image Cameron McCormack (:heycam) 2014-07-31 21:50:11 PDT
Comment on attachment 8465894 [details] [diff] [review]
patch 12 - Tests for transitions on elements that are reframing

Review of attachment 8465894 [details] [diff] [review]:
-----------------------------------------------------------------

Worth doing a dynamic change test on ::before/::after too?  And a test for an element that is changing from display:none to something else?
Comment 66 User image David Baron :dbaron: ⌚️UTC-8 2014-08-02 18:21:59 PDT
(In reply to Cameron McCormack (:heycam) from comment #61)
> Is the issue then that the new style context might not have the right parent
> (as that seems to be what nsCSSFrameConstructor::ResolveStyleContext does)?

The issue is that I'm hooking in to ResolveStyleContext in later patches, and in particular I need that hook to be called for all regular element style context resolution except for the root.  So I happen to be fine, but it seems like it's odd for the root to be the only regular element style context for a frame to be constructed not resolved through ResolveStyleContext.

(In reply to Cameron McCormack (:heycam) from comment #62)
> Comment on attachment 8465892 [details] [diff] [review]
> patch 10 - Check for difference in HasPseudoElementData before starting
> transitions
> ::: layout/style/nsTransitionManager.cpp
> @@ +146,5 @@
> > +    // bail.  We can't hit this codepath for normal style changes
> > +    // involving moving frames around the boundaries of these
> > +    // pseudo-elements since we don't call StyleContextChanged from
> > +    // ReparentStyleContext.  However, we can hit this codepath during
> > +    // the handling of transitions that start across reframes.
> 
> Can you give me a short example demonstrating when we do get in here?  If
> it's short enough maybe stick it in the comment here.

An example would be restyling something like:

<!DOCTYPE html>
<style>
div::first-line { color: green }
:link, :visited { color: inherit; transition: color ease 300ms }
</style>
<div>This paragraph has <a href="https://mozilla.org/">a link to Mozilla</a> in it.

with a restyle that:
  1. has the link *not* in the first line prior to the restyle (since I *think* that during frame construction we assume everything is in the first line and then fix it up during layout)
  2. reframes the div or one of its ancestors

This wasn't an issue before since restyling that doesn't involve reframes doesn't change whether anything matches ::first-letter or ::first-line.
Comment 67 User image Cameron McCormack (:heycam) 2014-08-12 00:31:46 PDT
Comment on attachment 8465892 [details] [diff] [review]
patch 10 - Check for difference in HasPseudoElementData before starting transitions

(In reply to David Baron [:dbaron] (UTC-7) (needinfo? for questions) from comment #66)
> An example would be restyling something like:

I guess that might be getting a bit long for a comment.  If you can easily turn this into a test, please do so.
Comment 68 User image David Baron :dbaron: ⌚️UTC-8 2014-08-13 15:01:35 PDT
(In reply to Cameron McCormack (:heycam) from comment #65)
> Worth doing a dynamic change test on ::before/::after too?  And a test for
> an element that is changing from display:none to something else?

I've added such tests (and factored the test code into a function). Note that the changing from display:none cases fail, but that's a separate bug.
Comment 69 User image David Baron :dbaron: ⌚️UTC-8 2014-08-13 15:30:56 PDT
(In reply to Cameron McCormack (:heycam) from comment #67)
> I guess that might be getting a bit long for a comment.  If you can easily
> turn this into a test, please do so.

I added a patch 13 with a test for that case.
Comment 72 User image David Baron :dbaron: ⌚️UTC-8 2014-08-14 08:48:33 PDT
*** Bug 915168 has been marked as a duplicate of this bug. ***
Comment 73 User image David Baron :dbaron: ⌚️UTC-8 2014-08-14 08:56:38 PDT
comment 71 shows that this has landed in central.  This means that :

 (1) this bug is fixed in today's nightly builds of Firefox.  (2014-08-14 builds)  See https://nightly.mozilla.org/ for getting nightly builds.

 (2) unless something you see news to the contrary in this bug, this should be fixed in Firefox 34.

Testing that the problems that led you here are now fixed in nightly is appreciated; it's much more useful to have that feedback now than after the fix ships.  That said, if there are other related issues that are still unresolved, those should be filed as separate bugs (if they're not filed already), though it might be worth noting them here.

Note that one related issue is bug 537143, on not supporting transitions in display:none subtrees.
Comment 74 User image David Baron :dbaron: ⌚️UTC-8 2014-08-14 10:36:14 PDT
*** Bug 788677 has been marked as a duplicate of this bug. ***
Comment 75 User image David Baron :dbaron: ⌚️UTC-8 2014-08-14 10:39:44 PDT
*** Bug 800927 has been marked as a duplicate of this bug. ***
Comment 76 User image David Baron :dbaron: ⌚️UTC-8 2014-08-14 10:41:54 PDT
*** Bug 821976 has been marked as a duplicate of this bug. ***
Comment 77 User image David Baron :dbaron: ⌚️UTC-8 2014-08-14 10:43:35 PDT
*** Bug 1011153 has been marked as a duplicate of this bug. ***
Comment 78 User image Jakub Chodorowicz 2014-08-18 10:20:01 PDT
It seems to be resolved in current nightly for me. Thanks!
Comment 79 User image Nicolas Hoffmann 2014-09-08 06:20:33 PDT
Hi,

don't know if it can help, I've made a test case while searching a solution for this bug: http://www.nicolas-hoffmann.net/bordel/divers/bug-transition-firefox/index.html

Anyway, some people confirm that it is fixed on last versions fo Firefox. (and buggy on Fx32)
Comment 80 User image Dustin Hoffmann 2014-09-21 07:08:43 PDT
Just came across this issue while deving (FF 32 stable) & can confirm it is indeed fixed for me in the current nightlies.
Comment 81 User image Kevin Brosnan [:kbrosnan] 2014-09-23 11:43:34 PDT
Release Note Request (optional, but appreciated)
[Why is this notable]: Fixed an often requested issue with CSS transitions
[Suggested wording]: I don't know the best way to describe this change
[Links (documentation, blog post, etc)]:

+Yoshino for Site Compatibility
Comment 82 User image Sylvestre Ledru [:sylvestre] 2014-10-03 03:10:56 PDT
Added in the release notes with "Fix glitches in CSS transitions" as wording.
Comment 83 User image David Baron :dbaron: ⌚️UTC-8 2014-10-03 10:03:37 PDT
(In reply to Sylvestre Ledru [:sylvestre] from comment #82)
> Added in the release notes with "Fix glitches in CSS transitions" as wording.

I think that's overly broad; could you either reword as "Fixed starting of CSS transitions that start together with changes to display, position, overflow, and similar properties" or drop from the release notes?

It does definitely belong in Firefox 34 for developers, though, so adding dev-doc-needed.  I have no idea why it belongs in the release notes, whose developer notes seem like a randomly-selected subset of Firefox 34 for developers.
Comment 84 User image Sylvestre Ledru [:sylvestre] 2014-10-03 12:18:06 PDT
> I think that's overly broad; could you either reword as "Fixed starting of
> CSS transitions that start together with changes to display, position,
> overflow, and similar properties" or drop from the release notes?
Updated! Thanks.

> It does definitely belong in Firefox 34 for developers, though, so adding
> dev-doc-needed.  I have no idea why it belongs in the release notes, whose
> developer notes seem like a randomly-selected subset of Firefox 34 for
> developers.
We try to highlight the items which seems the most important. However, this is subjective and it is why we ask for review on the release notes for each release.
Comment 85 User image Lawrence Mandel [:lmandel] (use needinfo) 2014-10-15 19:08:49 PDT
Used the following for the release note "CSS transitions start correctly when started at the same time as changes to display, position, overflow, and similar properties"
Comment 86 User image László Károlyi 2014-11-20 14:52:48 PST
This bug is still not fully fixed.

the link in https://bugzilla.mozilla.org/show_bug.cgi?id=625289#c32 demonstrates it well: http://jsfiddle.net/langbert/mDcQd/

if you set an overflow:hidden either on the body or the html element, the transitions doesn't work.
Comment 87 User image Martijn Wargers [:mwargers] 2014-11-20 14:57:52 PST
(In reply to laszlo from comment #86)
> This bug is still not fully fixed.
> 
> the link in https://bugzilla.mozilla.org/show_bug.cgi?id=625289#c32
> demonstrates it well: http://jsfiddle.net/langbert/mDcQd/
> 
> if you set an overflow:hidden either on the body or the html element, the
> transitions doesn't work.

Could you please file a new bug about that? And mention the bug number here? Thanks!
Comment 88 User image László Károlyi 2014-11-20 14:59:20 PST
(In reply to Martijn Wargers [:mwargers] (QA) from comment #87)
> (In reply to laszlo from comment #86)
> > This bug is still not fully fixed.
> > 
> > the link in https://bugzilla.mozilla.org/show_bug.cgi?id=625289#c32
> > demonstrates it well: http://jsfiddle.net/langbert/mDcQd/
> > 
> > if you set an overflow:hidden either on the body or the html element, the
> > transitions doesn't work.
> 
> Could you please file a new bug about that? And mention the bug number here?
> Thanks!

Sorry, I wasn't noticing that it'll be in FF34. Downloaded nightly, works there.
Comment 89 User image Jean-Yves Perrier [:teoli] 2014-11-28 03:44:41 PST
I've added the note in 
https://developer.mozilla.org/en-US/Firefox/Releases/34#CSS

Note You need to log in before you can comment on or make changes to this bug.