943858 - [SMS] Thread list starts scrolled to the top

Status: RESOLVED WORKSFORME

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

Description

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Normally when the user enters a sms thread the list is scrolled to the bottom. It seems like with APZ turned on that something weird is ongoing and the pan still starts from the top.

The funny thing is that I see the scrollbar at the bottom of the list when the thread is opened, so I wonder if there is a race somewhere or if the scrolling code always start from 0,0 ?

I'm also pretty sure this issue prevent me to find an other regression since the thread view has a scroll handler and there is probably something else broken here :)

Comment 1

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Seems resolved with the patches from bug 944047

Comment 2

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

I still see that. I wonder if this is related to https://github.com/mozilla-b2g/gaia/blob/master/apps/sms/js/thread_ui.js#L632 who tries to update the scroll position, and is out of sync with the async pan zoom code.

So the scrollPosition is updated synchronously from content and then async pan zoom send an event to scroll to a different position starting from the top, which is now very different.

Comment 3

[Julien Wajsberg [:julienw]]

This piece of code runs only when we get a scroll event and when we show another chunk of messages.

However, https://github.com/mozilla-b2g/gaia/blob/master/apps/sms/js/thread_ui.js#L637-L641 runs when you load a thread, so this might be it?

Comment 4

[Julien Wajsberg [:julienw]]

I am naively thinking that using scrollIntoView, which is a platform method, would work perfectly with whatever the platform is doing with scroll ;)

Comment 5

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

(In reply to Julien Wajsberg [:julienw] from comment #4)
> I am naively thinking that using scrollIntoView, which is a platform method,
> would work perfectly with whatever the platform is doing with scroll ;)

It should but there is likely a race somewhere :)

Comment 6

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

I think I know what's going on. Will try to provide a patch tomorrow.

Comment 7

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

OK. Something has landed in the last few days that fix this. Let's close it.