Closed Bug 952526 Opened 11 years ago Closed 11 years ago

[Messages] New sms appear outside the viewport/ below the screen

Categories

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

Other
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(blocking-b2g:1.3+, b2g18 unaffected, b2g-v1.1hd unaffected, b2g-v1.2 unaffected, b2g-v1.3 fixed, b2g-v1.3T fixed, b2g-v1.4 fixed)

RESOLVED FIXED
blocking-b2g 1.3+
Tracking Status
b2g18 --- unaffected
b2g-v1.1hd --- unaffected
b2g-v1.2 --- unaffected
b2g-v1.3 --- fixed
b2g-v1.3T --- fixed
b2g-v1.4 --- fixed

People

(Reporter: zcampbell, Assigned: gsvelto)

References

Details

(Keywords: dogfood, regression, Whiteboard: burirun1.3-2)

Attachments

(2 files)

STR 1. Grab a phone and a b2g phone 2. Send a message to the b2g phone 3. Open the SMS app in message thread view and leave it open 4. Send 5-6 more SMS to the b2g phone until thread view is filled Actual: The SMS app in thread view does not scroll down to the newest received message. As there is no visual cue (ie scroll bar) to signify more content outside the viewport it is not clear there is a new message. The status bar notification does not work when Messages app is open either so there is even less indication to the user that a new message has arrived. Expected Message app to scroll down to show the latest message at the bottom when a new message arrives This bug is at least 12 days old as I found it on my dogfooding phone which has an almost 2 week old build on it. I have also verified it on today's build. Gaia 574f79512a7b8a9ab99211b16a857ab812d7994e Gecko http://hg.mozilla.org/mozilla-central/rev/599100c4ebfe BuildID 20131220040201 Version 29.0a1
So, that's expected we don't scroll, but you should see a notice that looks like https://bug905208.bugzilla.mozilla.org/attachment.cgi?id=810514 instead (implemented in v1.3). Do you see this notice sometimes? Is there a specific situation that you don't see it?
Flags: needinfo?(zcampbell)
Ok, I get it, we don't get the notice if we havent scrolled at all yet. And this happens if the thread is too small to scroll when we open the thread.
Flags: needinfo?(zcampbell)
Whiteboard: burirun1.3-2
Moving over flags from the dupe.
Assignee: nobody → schung
blocking-b2g: --- → 1.3?
blocking-b2g: 1.3? → 1.3+
Keywords: dogfood
Note that we did scroll to the new message in the past unless the user had explicitly scrolled up. The behavior should basically be: - If the user is already at the bottom we display the new message and scroll it into view - If the user has manually scrolled up we assume he/she is reading some old message and we don't want to distract him/her by scrolling down so we display a notification instead Looking at the changelog it seems to me that we regressed in bug 929919, see this change that removed scrolling to the new message: https://github.com/mozilla-b2g/gaia/commit/8115e89f4bd5b1f3cb68545f48aa20bb2d7dbe07#diff-74841e428dea67f7357f1893a4d26155L484 Taking this, I'll try and cook up a fix, it should be pretty easy.
Assignee: schung → gsvelto
Status: NEW → ASSIGNED
Blocks: 929919
This patch fixes the problem and introduces a couple of unit tests to make sure we don't regress anymore on this behavior.
Attachment #8376698 - Flags: review?(felash)
Summary: New sms appear outside the viewport/ below the screen → [Messages] New sms appear outside the viewport/ below the screen
Comment on attachment 8376698 [details] [diff] [review] [PATCH] Fix automatic scrolling of the message view when a new SMS is received Review of attachment 8376698 [details] [diff] [review]: ----------------------------------------------------------------- Taking this review to lighten the load—everything looks good to me, nice and easy fix. Works well on device. Good tests :) r+
Attachment #8376698 - Flags: review?(felash) → review+
(In reply to Rick Waldron [:rwaldron] from comment #8) > Taking this review to lighten the load—everything looks good to me, nice and > easy fix. Works well on device. Good tests :) r+ Thanks a lot Rick! The Travis run was green: https://travis-ci.org/mozilla-b2g/gaia/builds/18928051 so pushed to gaia/master 61a9ba7dd6006a21a8bde4fd4c5d52c23caebf78
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Comment on attachment 8376698 [details] [diff] [review] [PATCH] Fix automatic scrolling of the message view when a new SMS is received [Approval Request Comment] [Bug caused by] (feature/regressing bug #): regressing bug 929919 [User impact] if declined: if the user receives a new message in the thread he's looking at, the view won't be scrolled down to show him the new message if he's already at the bottom of the thread (which was the standard behavior prior to bug 929919) [Testing completed]: Landed and tested on master using both a device and the emulator, unit-tests were added to cover this scenario [Risk to taking this patch] (and alternatives if risky): Practically none, it's a one-line fix which adds back a line that was erroneously removed [String changes made]: none
Attachment #8376698 - Flags: approval-gaia-v1.3?(fabrice)
Thanks to you both :)
Attachment #8376698 - Flags: approval-gaia-v1.3?(fabrice) → approval-gaia-v1.3+
v1.3: 38519ceb6ab5c46c35a3fae3a70f5f26e4b3c59c
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: