830222 - Opening an SMS thread scrolls through full history of SMS messages in thread

Status: RESOLVED FIXED

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

Description

[Jared Wein [:jaws] (please needinfo? me)]

This is in build 20130110070201 on the beta channel.

When entering an SMS thread, the last message is displayed and then the viewport jumps to the first message and scrolls down to the last message.

This is not 100% reproducible, but it is quite jarring and annoying when it happens.

I still want to log this bug in case others are experiencing it and in the hopes that we will get better STR.

Comment 1

[Jared Wein [:jaws] (please needinfo? me)]

These steps are reproducible for me 100% of the time.

STR:
Open the SMS app and enter a thread that is longer than the viewport.
Press the home button
Re-enter the SMS app

Upon reentering the SMS app, the previously selected thread will be scrolled from top to bottom.

Comment 2

[Jason Smith [:jsmith]]

Basecamp? has been retired. Please use tef? and tracking-b2g?

Comment 4

[Etienne Segonzac (:etienne)]

Currently *every* time the SMS app becomes visible we re-render all the threads and the current conversation.

I'm attaching a patch that fixes the |updateTimeHeaders| function, allowing us to avoid all this re-rendering.

Comment 5

[Etienne Segonzac (:etienne)]

Attachment: Patch proposal

Testing completed: dogfooded

Comment 6

[Borja Salguero [:borjasalguero]]

Hi Etienne,
I was checking your code, but I dont know why we have to remove 'onVisibilityChange'. I know what you are trying to solve, but 'visibility' event it's needed in order to refresh the info of a thread.
For example, imagine the following scenario:
- Open SMS App, and go to thread '[phone]'
- Open Contacts
- Create contact 'Robert' with '[phone]'
- Go back to SMS App

With this change in the visibility, we could check again if the info of the contact is up-to-date. Removing this visibility management, we are loosing this functionality.

Comment 7

[Etienne Segonzac (:etienne)]

(In reply to Borja Salguero [:borjasalguero] from comment #6)
> Hi Etienne,
> I was checking your code, but I dont know why we have to remove
> 'onVisibilityChange'. I know what you are trying to solve, but 'visibility'
> event it's needed in order to refresh the info of a thread.
> For example, imagine the following scenario:
> - Open SMS App, and go to thread '[phone]'
> - Open Contacts
> - Create contact 'Robert' with '[phone]'
> - Go back to SMS App
> 
> With this change in the visibility, we could check again if the info of the
> contact is up-to-date. Removing this visibility management, we are loosing
> this functionality.

Updating the patch right away!

Comment 8

[Borja Salguero [:borjasalguero]]

Etienne, Im gonna take the review of this patch ok? I would like to test it once you have the fix of contacts!

Comment 9

[Etienne Segonzac (:etienne)]

(In reply to Borja Salguero [:borjasalguero] from comment #8)
> Etienne, Im gonna take the review of this patch ok? I would like to test it
> once you have the fix of contacts!

np, wasn't sure you were back from PTO.

Comment 10

[Etienne Segonzac (:etienne)]

Attachment: Patch v2

Note: this patch is meant to be a surgical as possible.

Comment 11

[Borja Salguero [:borjasalguero]]

Thanks etienne! This patch fix the annoying effect of re-rendering. Making some test I've discovered a small issue while editing SMSs, but I've filed a new bug for it https://bugzilla.mozilla.org/show_bug.cgi?id=833010. Thanks!

Comment 12

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

https://github.com/mozilla-b2g/gaia/commit/bd8ad2a34f0ac3fa7e1a0da6dbde89b973ec537d