Page MenuHomePhabricator

Lazy loaded images animation doesn't trigger until scrolling stops in mobile browsers
Closed, ResolvedPublic2 Story Points

Description

Per comments and text in T135430, it seems like the loaded image animation does not trigger until the scrolling stops on mobile browsers (reproduced both on Android chrome and iOS safari).

The animation and the image appearing should happen even if the user is scrolling with their finger down, not on scroll stop (or lifting the finger).

Some possible causes for the behavior:

  • We're using mw.mobileFrontend.on('scroll') which is debounced. It may be the case that the debounced scroll event doesn't get triggered until the scroll finishes, and that's why this behavior is being observed. A solution may be switching to listening to a *throttled* scroll instead of a *debounced*.
  • It may be that the browser is not triggering CSS animations until scrolling stops (needs verification). Switching to a CSS transition would fix this if the premise was true. Be careful with CSS transitions since triggering them reliably across browsers so that the animation always happens has proven to be tricky (that's why we switched to css animations for this, see T135430#2378887 for more info).

AC:

  • Verify the issue still exists (since T133565 was resolved)
  • While scrolling, lazy images get loaded and animated in, even when the scrolling never stops (scrolling finger always down)

Related Objects

StatusAssignedTask
OpenNone
OpenNone
OpenNone
OpenNone
OpenNone
OpenNone
OpenNone
DeclinedNone
OpenNone
Resolveddr0ptp4kt
DuplicateJhernandez
Duplicatedr0ptp4kt
OpenNone
ResolvedJdlrobson
ResolvedJdlrobson
ResolvedJhernandez
ResolvedJhernandez
DuplicateNirzar
Resolveddr0ptp4kt

Event Timeline

Restricted Application added subscribers: Zppix, Aklapper. · View Herald TranscriptJun 15 2016, 9:45 AM

Note that T133565 Images do not load until scroll event completes (not during scroll) on iOS) seems to be resolved, so this bug may already be solved if that bug is really fixed.

Needs verification.

Jhernandez updated the task description. (Show Details)Jun 15 2016, 10:04 AM
Jhernandez added a subscriber: Jdlrobson.

Note that @Jdlrobson told me in chat that he was experiencing this bug in android chrome yesterday.

@dr0ptp4kt this may be interesting to tackle before the end of the quarter given the deployments next sprint. Not sure how to fit it though.

jhobs triaged this task as High priority.Jun 15 2016, 5:12 PM
jhobs added a subscriber: jhobs.

@Jhernandez @dr0ptp4kt does this need to go into the current sprint since it blocks the task in the sprint, or are we de-scoping the other task and this can go in the next sprint?

@jhobs, I believe it's the other way around - this task is presently marked as blocked by the task that's in the sprint.

I'd rather we hold off on trying to work this task until we're totally sure whether we want to handle the aggressive scrolling case and, if so, how.

T133565: Images do not load until scroll event completes (not during scroll) on iOS accounted for reasonable scrolling, but not for the rapid scrolling case.

I'll get something scheduled with @Nirzar, @Krinkle, @Jhernandez, and @Peter so we can work out the details. It may be we'd want to place this in sprint 75, but maybe later - we'll see how the timing and complexity play out.

I've been trying to reproduce the bug on Android 6, Chrome 48 and FF mobile and I can't manage to reproduce, with scrolling and my finger always down the images still load and fade in when loaded.

It would be good to know if somebody else has specific steps to reproduce.

I'll see if I can try it on iOS later.

After chatting in a meeting, it seems like debounce is a pretty bad choice for listening to the scroll events here.

I'm going to try throttling instead and will ping people to see if the experience is better.

For the groomers, feel free to move to next sprint or triaged but future.

Change 294895 had a related patch set uploaded (by Jhernandez):
Use throttled events for lazy loading images

https://gerrit.wikimedia.org/r/294895

Email sent, patch deployed in staging:

Hi,
As promised, visit:
https://reading-web-staging.wmflabs.org/wiki/Elvis_Presley?useformat=mobile&mobileaction=beta
Lazy loaded images should be a lot more responsive, try it out and see how it feels.
If it feels right I'll ask for code review and merge it to the repo.
Patch deployed: https://gerrit.wikimedia.org/r/#/c/294895/
Cheers

Looked good on Asus Nexus 7, LG Nexus 4, and iPhone 5c.

Windows Phone 7.5 and non-compression Opera Mini behaved the same, roughly speaking - worked, but not as pre-emptive. I think this is fine.

Compression Opera Mini, as expected because nothing additional was touched, loaded in all the images as usual, as I understand via NORLQ script.

Crazy fast scrolling could overshoot images before they were downloaded, but that's fine.

I noticed things were ref lowing again, though? Is that just a symptom of not all code being merged together or something? Assuming the reflow is resolved, I'd be happy to see the update merged to move us into a better position with this.

I moved this to code review in 75.

phuedx added a subscriber: phuedx.Jun 22 2016, 9:20 AM

I noticed things were ref lowing again, though? Is that just a symptom of not all code being merged together or something? Assuming the reflow is resolved, I'd be happy to see the update merged to move us into a better position with this.

In which browsers/on which devices were you seeing reflowing? Zero/one/infinity?

It was iOS Safari. iPhone 5c iOS 9.3 as far as I recall (although it may have been the iOS simulator for an iPhone 4s; mixing up which tests I did where several days back now). Would you please rebase and land on reading-web-staging and advise what you see on your devices for the king of rock article? I'll also check once confirmed it's on reading-web-staging with my devices.

@dr0ptp4kt Could you confirm that you're not seeing a reflow on your devices now? I haven't checked out @Jhernandez's change yet as I'd like to make sure that it's not a pre-existing bug – one that we'd have to reopen T135430 for.

I can't see anything wrong in Safari or iOS safari 9.3 iphone 5S on master. I'll try with the patch in staging soon.

Same with the patch from this task. I've checked it out into staging rebased on top of current master.

I'll keep it there today so that you can try and reproduce @dr0ptp4kt (I'll send an email about it too)

Meanwhile, intrepid developers, feel free to +2 it. It is a tiny patch.

I no longer see the reflow. Consider this my signoff.

Change 294895 merged by jenkins-bot:
Use throttled events for lazy loading images

https://gerrit.wikimedia.org/r/294895

Jhernandez set the point value for this task to 2.Jun 30 2016, 9:01 AM