bugzilla.mozilla.org has resumed normal operation. Attachments prior to 2014 will be unavailable for a few days. This is tracked in Bug 1475801.
Please report any other irregularities here.

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

RESOLVED FIXED

Status

Firefox OS
Gaia::SMS
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: zac, Assigned: gsvelto)

Tracking

({dogfood, regression})

unspecified
Other
Gonk (Firefox OS)
dogfood, regression
Dependency tree / graph

Firefox Tracking Flags

(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)

Details

(Whiteboard: burirun1.3-2)

Attachments

(2 attachments)

(Reporter)

Description

5 years ago
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
(Reporter)

Updated

5 years ago
Blocks: 949551
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)

Updated

5 years ago
Whiteboard: burirun1.3-2

Updated

5 years ago
Duplicate of this bug: 972654
Moving over flags from the dupe.
Assignee: nobody → schung
blocking-b2g: --- → 1.3?
Keywords: regression, regressionwindow-wanted

Updated

5 years ago
blocking-b2g: 1.3? → 1.3+

Updated

5 years ago
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

Updated

5 years ago
Blocks: 929919

Updated

5 years ago
Keywords: regressionwindow-wanted
Created attachment 8376698 [details] [diff] [review]
[PATCH] Fix automatic scrolling of the message view when a new SMS is received

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)
Created attachment 8376699 [details] [review]
[PULL REQUEST] Fix automatic scrolling of the message view when a new SMS is received
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
Last Resolved: 5 years ago
status-b2g18: --- → unaffected
status-b2g-v1.1hd: --- → unaffected
status-b2g-v1.2: --- → unaffected
status-b2g-v1.3: --- → affected
status-b2g-v1.3T: --- → affected
status-b2g-v1.4: --- → fixed
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
status-b2g-v1.3: affected → fixed
Duplicate of this bug: 949113

Updated

4 years ago
Duplicate of this bug: 976494
https://github.com/mozilla-b2g/gaia/commit/38519ceb6ab5c46c35a3fae3a70f5f26e4b3c59c - merged on 1.3t as well.
status-b2g-v1.3T: affected → fixed
You need to log in before you can comment on or make changes to this bug.