Closed Bug 823460 Opened 7 years ago Closed 7 years ago

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

Categories

(Firefox OS Graveyard :: General, defect)

defect
Not set

Tracking

(blocking-basecamp:+, firefox19 fixed, firefox20 fixed, b2g18 fixed)

RESOLVED FIXED
B2G C3 (12dec-1jan)
blocking-basecamp +
Tracking Status
firefox19 --- fixed
firefox20 --- fixed
b2g18 --- fixed

People

(Reporter: timdream, Assigned: nrc)

Details

Attachments

(1 file, 1 obsolete file)

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?
Additional note:

If slow transition is set, closing animation will always happen w/o opacity, and opening animation will always happen with opacity.
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
nrc, can you reproduce this?
Flags: needinfo?(ncameron)
(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.
Flags: needinfo?(ncameron)
Assignee: nobody → ncameron
blocking-basecamp: ? → +
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.
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.
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...
Attached patch patch (obsolete) — Splinter Review
Attachment #695274 - Flags: review?(dbaron)
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
Attachment #695274 - Flags: review?(dbaron) → review+
Whiteboard: [leave-open]
Attached patch patchSplinter Review
addressed comments; carrying r=dbaron
Attachment #695274 - Attachment is obsolete: true
Attachment #695278 - Flags: review+
Tim: could you confirm if my patch fixes the problem before we close the bug please?
Flags: needinfo?(timdream+bugs)
Target Milestone: --- → B2G C3 (12dec-1jan)
Yeah it works, thanks.
Status: NEW → RESOLVED
Closed: 7 years ago
Flags: needinfo?(timdream+bugs)
Resolution: --- → FIXED
Whiteboard: [leave-open]
(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) from comment #16)
> Yeah it works, thanks.

Great! Thanks for testing this.
You need to log in before you can comment on or make changes to this bug.