Swiping an app to close it is janky and doesn't stop horizontal scrolling

RESOLVED WORKSFORME

Status

defect
RESOLVED WORKSFORME
4 years ago
4 years ago

People

(Reporter: cwiiis, Assigned: mcav)

Tracking

(Depends on 1 bug, {foxfood, polish, regression})

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [systemsfe])

Attachments

(1 attachment)

Reporter

Description

4 years ago
If you drag an app up to close it, it doesn't stop horizontal scrolling, which looks weird. Furthermore, it's also quite janky.

I suggest that after passing the vertical drag threshold, overflow-x should be set to hidden on the scrolling container - this will disable horizontal dragging.

Furthermore, if it isn't already, will-change: transform should also be set on the dragged app.

---

I was also pondering a technique that would be even smoother, whereby you use sticky positioning to set all apps except the centre app as fixed-position on the vertical axis, so you can just rely on vertical scroll position to swipe-close an app. This would be a lot more responsive and you could just rely on axis-lock to determine whether you're swiping horizontally or vertically (you'd likely still need the overflow-x trick above to counteract diagonal scrolling) - this could be a fun thing to experiment with, but the quick fixes are listed above.
Is this is a regression in the recent dogfood OTA update (Build ID 20150707033152)? The task manager seemed OK (to me) until my foxfood phone OTA updated.
Keywords: foxfood, regression
(In reply to Chris Peterson [:cpeterson] from comment #1)
> Is this is a regression in the recent dogfood OTA update (Build ID
> 20150707033152)? The task manager seemed OK (to me) until my foxfood phone
> OTA updated.

Yeah, kind-of: the latest OTA pulled in bug 1161229, which improves the task-manager performance in lots of ways, but not the swipe-to-delete performance - which was not improved and reportedly sometimes a little worse. We have bug 1177823 on file which should improve it a little. I'm also interested in :cwiis' ideas though and would love to see a patch or prototype if anyone has time before I can get to it.
Marcus, do you want to start with this? You might also pull in or dupe bug 1177823. I have a patch underway for 1169012 that touches this code, but I would rather rebase it than block progress on this.
Flags: needinfo?(m)
Duplicate of this bug: 1177395
Captured a profile on Nexus5 which drags the app up and down repeatedly.

  http://people.mozilla.org/~bgirard/cleopatra/#report=a08748785087738954cd2da85bb33f5c060fd614

Gecko: cb8bdb8ffaef
Gaia: ef691bb7e97bdbf709c3778c64363998be769a70
(In reply to Ting-Yu Chou [:ting] from comment #5)
> Captured a profile on Nexus5 which drags the app up and down repeatedly.

Card.onCrossSlide() changes list item's translateY, but I am not sure why there're build displaylist/layer calls.
- Calling preventDefault() in onCrossSlide() doesn't prevent horizontal scroll, since preventDefault() works only if it is touchstart/touchend or the first touchmove event [1]. But it's difficult to determine user is swiping horizontally or vertically at the first touchmove event.

- The dragging goes much smoother if preventDefault() whenever receiving a touchmove event. But it gets jankier when there're more cards.

[1] https://dxr.mozilla.org/mozilla-central/source/layout/base/nsPresShell.cpp#8058
(In reply to Ting-Yu Chou [:ting] from comment #6)
> Card.onCrossSlide() changes list item's translateY, but I am not sure why
> there're build displaylist/layer calls.

While dragging, each nsRefreshDriver::Tick() triggers nsViewManager::ProcessPendingUpdates() since there're animation restyles (AFAIK those are CSS transitions) for elements such as:

  #lockscreen-panel-main,
  #lockscreen-masked-background,
  #AppChromeX

And there could be more after dragging more different apps' card view, I am still trying to figure out why is this happening.
dbaron, I noticed css transitions will be added to pending restyle by CommonAnimationManager::AddStyleUpdatesTo() even:

  1) the element is not visible on screen (though css display is not none and visibilty is not hidden), and
  2) the transition property is not altered

Is this expected or a bug? Thank you.
Flags: needinfo?(dbaron)
Depends on: 1189598
Clear NI since I filed bug 1189598.
Flags: needinfo?(dbaron)
No longer blocks: 1094010
Comment on attachment 8641955 [details] [review]
[gaia] sfoster:taskmanager-crossslide-bug-1180168 > mozilla-b2g:master

A quick patch to address the side-scrolling while swiping up issue. I'm a bit worried toggling overflow-x mid-drag might trigger reflow and more jank, but the side-to-side stability is a net improvement I think. 

If this looks reasonable I suggest we move the other jank issues mentioned in this bug to other/new bugs and land to get in fixes incrementally
Attachment #8641955 - Flags: review?(etienne)
Assignee: nobody → sfoster
Comment on attachment 8641955 [details] [review]
[gaia] sfoster:taskmanager-crossslide-bug-1180168 > mozilla-b2g:master

Made a few comments on github but I think it's the right approach.

What's killing us (I think) is not the reflow but a huge repaint when we switch the overflowX to hidden, not much we can do about it sadly.
Attachment #8641955 - Flags: review?(etienne) → feedback+
Re-assigning to :mcav as his WIP patch in bug 1206928 appears to fix this
Assignee: sfoster → m
Flags: needinfo?(m)
Assignee

Comment 15

4 years ago
Fixed in bug 1206928.
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.