1031287 - After the first swipe in the task manager, all subsequent swipes have weird input lag

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia::System, defect)

Description

[Chris Lord [:cwiiis]]

I think removing the transition from the task cards isn't working correctly, so after the first drag/swipe, all subsequent ones lag behind the input events and feel weird. Should be a reasonable easy fix so taking it.

Comment 1

[Chris Lord [:cwiiis]]

Attachment: Fix removing transitions from cards in task manager

Two problems, applying a MozTransition style of undefined doesn't do anything, and I don't think there's any need to use this.prev/nextCard when we have the exact elements the styles were applied to in scope.

Comment 2

[Sam Foster [:sfoster] (he/him)]

See drive-by comments on github. I'm not saying transition: none is the wrong thing to do, just pointing out the meaning of that undefined value and how it is handled in AppWindow.prototype.applyStyle

Comment 3

[Chris Lord [:cwiiis]]

(In reply to Sam Foster [:sfoster] from comment #2)
> See drive-by comments on github. I'm not saying transition: none is the
> wrong thing to do, just pointing out the meaning of that undefined value and
> how it is handled in AppWindow.prototype.applyStyle

You're right, setting it to "" I think would have the right effect, assuming it works - I'll try that on Monday. That said, we're overriding it entirely, it'd be a bit odd to do that if we couldn't assume full control of it (so I think none is ok too, but I'll see if "" works in case things change in the future).

Comment 4

[Jason Smith [:jsmith]]

A video would help understand the user impact here.

Comment 5

[(inactive after 6/18) Alive Kuo [:alive]]

Do we still have the open/close animation after clearing the style.transition in javascript?

Comment 6

[Chris Lord [:cwiiis]]

(In reply to Alive Kuo [:alive][NEEDINFO!] from comment #5)
> Do we still have the open/close animation after clearing the
> style.transition in javascript?

Yes, these still work fine.

I've just force-pushed to replace 'none' with '', which works but also doesn't override the transition style in the stylesheet. This is ready for review.

Comment 7

[(inactive after 6/18) Alive Kuo [:alive]]

Comment on attachment 8447152 [details] [review]
Fix removing transitions from cards in task manager

It's better to have a test.

Comment 8

[Chris Lord [:cwiiis]]

(In reply to Alive Kuo [:alive][NEEDINFO!] from comment #7)
> Comment on attachment 8447152 [details] [review]
> Fix removing transitions from cards in task manager
> 
> It's better to have a test.

Fair enough, what part would you like to test here?

Comment 9

[Chris Lord [:cwiiis]]

needinfo'ing myself again too to provide a video.

Comment 10

[(inactive after 6/18) Alive Kuo [:alive]]

(In reply to Chris Lord [:cwiiis] from comment #8)
> (In reply to Alive Kuo [:alive][NEEDINFO!] from comment #7)
> > Comment on attachment 8447152 [details] [review]
> > Fix removing transitions from cards in task manager
> > 
> > It's better to have a test.
> 
> Fair enough, what part would you like to test here?

We have a discussion on IRC, clearing needinfo

Comment 11

[Chris Lord [:cwiiis]]

Comment on attachment 8447152 [details] [review]
Fix removing transitions from cards in task manager

Now with test (confirmed fails without the patch applied).

Comment 12

[Eric Kramer [:Kramer]]

Added video URL ( http://youtu.be/nfu4FnH8q8Y ). The input lag is most noticeable when holding a card and moving it slowly. 

Checked on 2.0:
Flame 2.0
BuildID: 20140627000201
Gaia: 8df02268fcd7e80c5fab8c1ec099772e37f8659d
Gecko: 731a5e8831e6
Version: 32.0a2 (2.0) 
Firmware Version: v122
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0

Comment 13

[(inactive after 6/18) Alive Kuo [:alive]]

Comment on attachment 8447152 [details] [review]
Fix removing transitions from cards in task manager

Please rebase your commit into only one.

Comment 14

[Chris Lord [:cwiiis]]

Attachment: Fix removing transitions from cards in task manager

Squashed commits, carrying f+/r+

Comment 15

[Chris Lord [:cwiiis]]

Merged into master: https://github.com/mozilla-b2g/gaia/commit/feb3e61f956d50b4e4b326f9ee32e9ae98d19629

Comment 16

[Chris Lord [:cwiiis]]

Video: https://www.dropbox.com/s/uwicg1qkap74zkb/task-manager%20input%20lag.3gp

Note how the first swipes track quite well but after I lift my finger and replace it, there's lag.

Comment 17

[Chris Lord [:cwiiis]]

Comment on attachment 8449298 [details] [review]
Fix removing transitions from cards in task manager

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): Laggy task manager
[User impact] if declined: See comment #15
[Testing completed]: Been on master for a day, has test coverage
[Risk to taking this patch] (and alternatives if risky): Low risk - the biggest risk is that the situation that already exists remains (but tests and manual testing show it to be fixed)
[String changes made]: None

Comment 18

[Ryan VanderMeulen [:RyanVM]]

v2.0: https://github.com/mozilla-b2g/gaia/commit/4caca2282560f1f9658e078c306d14fa0377e5e7

Comment 19

[Shally Li（leave from Mozilla）]

This issue has been successfully verified on Flame 2.1&2.0.
See attachment: verified_v2.1.mp4
Reproducing rate: 0/5

Flame 2.1 build:
Gaia-Rev        ccb49abe412c978a4045f0c75abff534372716c4
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/18fb67530b22
Build-ID        20141202001201
Version         34.0
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20141202.034824
FW-Date         Tue Dec  2 03:48:34 EST 2014
Bootloader      L1TC00011880

Flame 2.0 build:
Gaia-Rev        8d1e868864c8a8f1e037685f0656d1da70d08c06
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/c756bd8bf3c3
Build-ID        20141202000201
Version         32.0
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20141202.034707
FW-Date         Tue Dec  2 03:47:18 EST 2014
Bootloader      L1TC00011880

Comment 20

[Shally Li（leave from Mozilla）]

Attachment: verified_v2.1.MP4