823460 - Opacity transition doesn't happen all the time for window open/closing transition

Status: RESOLVED FIXED

(Firefox OS Graveyard :: General, defect)

Description

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Should be a blocking+ bug from gal.

STR:

1. open an app / close an app
2. observe it's transition

Expected:

Transition follows CSS instructions at [1] and [2].
Closing transition should transitioned to [3]

Actual:

Transition omits the opacity transition.

Note:

1) not an issue "switch-app" class rules -- I have verified the rules there did not applied to non-app-to-app transition.
2) Remove the 0.2s opacity delay at [2] doesn't help.

turn on "slow transition" might help: https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/window_manager.js#L72


[1] https://github.com/mozilla-b2g/gaia/blob/master/apps/system/style/system/system.css#L276
[2] https://github.com/mozilla-b2g/gaia/blob/master/apps/system/style/system/system.css#L326
[3] https://github.com/mozilla-b2g/gaia/blob/master/apps/system/style/system/system.css#L271


Chris, is there anything else I could investigate further?

Comment 1

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Additional note:

If slow transition is set, closing animation will always happen w/o opacity, and opening animation will always happen with opacity.

Comment 2

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Additional note #2:

Opacity transition is intentionally disabled for cards view and switching card closing transition; when it took place, the frame flashes before being removed. However, we setVisible() the frame to false and it's CSS visibility in the same function.

https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/window_manager.js#L433-437

Comment 3

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

nrc, can you reproduce this?

Comment 4

[Nick Cameron [:nrc]]

(In reply to Chris Jones [:cjones] [:warhammer] from comment #3)
> nrc, can you reproduce this?

Yes, I can reproduce on phone and desktop FF. Will look into the problem.

Comment 5

[Nick Cameron [:nrc]]

I think I can only repro this (on desktop) with a delay on the opacity, which contradicts note 2 in the bug description. So I'm not even sure if it is the same bug. I can't try different things on my phone at the moment, sorry.

Comment 6

[Nick Cameron [:nrc]]

Oh, well actually, it seems to run fine the first time even with a delay, but then running my test page again gets screwed up. So maybe there is something getting stored somewhere it should get removed, which would explain why removing the delay may or may not help.

Comment 7

[Nick Cameron [:nrc]]

It turns out transitions with delays which get OMTAed are broken. When the transition is added, we try to add the transition to a layer, and if the transition is throttled OMTA then the transition manager considers its job done. But, if there is a delay on the transition then the layer ignores the transition. And it never starts.

I see two solutions: either we add delayed transitions to their layer straight away and add a segment with no animation for the delay, or we trip a flag when we start an animation and detect in the transition manager when the transition should start and if the flag is not tripped, then we flush so the transition gets added to the layer. The latter sounded easier, so I did that. Patch coming up...

Comment 8

[Nick Cameron [:nrc]]

Attachment: patch

Comment 9

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

Comment on attachment 695274 [details] [diff] [review]
patch

Please rename mIsRunning to mIsRunningOnCompositor

Also, please update the comment:
  // Trim transitions that have completed, and post restyle events for
  // frames that are still transitioning.
near the start of FlushTransitions to also mention that you need to start transitions with delays.

r=dbaron with that

Comment 10

[Nick Cameron [:nrc]]

Attachment: patch

addressed comments; carrying r=dbaron

Comment 11

[Nick Cameron [:nrc]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b81ccb796b72

Comment 12

[Nick Cameron [:nrc]]

Tim: could you confirm if my patch fixes the problem before we close the bug please?

Comment 13

[Nick Cameron [:nrc]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/3b73a67f3df3

Comment 14

[Nick Cameron [:nrc]]

https://hg.mozilla.org/releases/mozilla-b2g18/rev/996e3c6e4260

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/b81ccb796b72

Comment 16

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Yeah it works, thanks.

Comment 17

[Nick Cameron [:nrc]]

(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) from comment #16)
> Yeah it works, thanks.

Great! Thanks for testing this.