Closed Bug 1042887 Opened 6 years ago Closed 6 years ago

Cannot scroll in messages app

Categories

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

defect
Not set
blocker

Tracking

(blocking-b2g:2.1+, b2g-v2.0 unaffected, b2g-v2.1 verified)

VERIFIED FIXED
2.1 S1 (1aug)
blocking-b2g 2.1+
Tracking Status
b2g-v2.0 --- unaffected
b2g-v2.1 --- verified

People

(Reporter: ich, Assigned: azasypkin)

References

(Blocks 1 open bug)

Details

(Keywords: regression, Whiteboard: [c=regression p= s= u=][sms-sprint-2.1S1])

Attachments

(4 files)

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:30.0) Gecko/20100101 Firefox/30.0 (Beta/Release)
Build ID: 20140611075517

Steps to reproduce:

1. Open the "Messages" app.
2. Open a thread which is longer than the screen is high.


Actual results:

You cannot scroll the content of the thread.  The message input is not shown.


Expected results:

The content should be scrollable and the message input should be shown.
Environment:

Gaia      5458f73e319759543fddf7e96d7ece4d78318e32
BuildID   20140723193600
Version   34.0a1
ro.build.version.incremental=eng..20140505.045022
ro.build.date=Mon May  5 04:51:06 CST 2014
I also encountered this issue in today's 2.1 build.

Environmental Variables:
Device: Flame Master
Build ID: 20140723040201
Gaia: 01d86c8cc615658694b114ca5711763efc4ef205
Gecko: d0f6259e8446
Version: 34.0a1 (Master)
Firmware Version: v122
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.

This issue did NOT occur in the 2.0 build.

Environmental Variables:
Device: Flame 2.0
Build ID: 20140723000201
Gaia: bf3fb88039843359d0a5759b2c0fb787abae544f
Gecko: 57c44a3f83f6
Version: 32.0 (2.0) 
Firmware Version: v122
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0
Status: UNCONFIRMED → NEW
QA Whiteboard: [QAnalyst-Triage?]
Ever confirmed: true
Flags: needinfo?(pbylenga)
This issue still occurred after disabling APZ.
[Blocking Requested - why for this release]:
2.0 to 2.1 regression of a core feature.

Requesting a regression window.
blocking-b2g: --- → 2.1?
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(pbylenga)
I can confirm the issue.

Found on:
Alcatel One Touch Fire production (got from T-mobile Poland)
B2G version: 2.1.0.0-prerelease master
Platform version: 34.0a1
Build Identifier: 20140723040201
Git commit info: 2014-07-23 05:51:38 01d86c8c
QA Contact: jmitchell
Could be another regression of bug 1022612.

Waiting for the regression window.
Duplicate of this bug: 1042980
This breaks usability. It should be a blocker.
blocking-b2g: 2.1? → 2.1+
Whiteboard: [c=regression p= s= u=]
Mozilla Inbound Regression Window:

Last Working:
Device: Flame Master
Build ID: 20140722080925
Gaia: 649245c238a043af32acb109b2613f578323f8e1
Gecko: f604cdbc6e85
Version: 34.0a1 (Master)
Firmware Version: v122
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

First Broken:
Device: Flame Master
Build ID: 20140722082826
Gaia: 649245c238a043af32acb109b2613f578323f8e1
Gecko: b489ff052163
Version: 34.0a1 (Master)
Firmware Version: v122
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Last Working Gaia First Broken Gecko: Repro (indicating Gecko issue)
Gaia: 649245c238a043af32acb109b2613f578323f8e1
Gecko: b489ff052163

First Broken Gaia Last Working Gecko: No Repro
Gaia: 649245c238a043af32acb109b2613f578323f8e1
Gecko: f604cdbc6e85

Gecko pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=f604cdbc6e85&tochange=b489ff052163
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(ktucker)
Daniel - this pushlog is all your work, can you take a look?
Flags: needinfo?(dholbert)
Yup, I'll take a look. It's likely we just need a "min-height:0" somewhere.

(That pushlog was primarily about changing the default min-sizing behavior of flex items [which used to default to 0, but now doesn't necessarily], in response to a spec change.  I's expected that it'll break some content that uses flexbox, but hopefully not too much content, and in ways that should be workaroundable.)
Flags: needinfo?(dholbert)
Flags: needinfo?(dholbert)
I'm trying to reproduce in a desktop b2g build, and I did hit related bug 1043518, but I haven't been able to trigger this bug yet because I can't "view a thread which is longer than the screen is high" (from comment 0) because I can't send any messages, because the send-message button has no effect.

So, I have no threads.

Any way around this? I'm guessing this is because my desktop machine does not have a cell plan :) but I'd think we'd have a way of fake-sending SMS messages for testing... (I did build my gaia profile with DESKTOP_SHIMS=1, if it matters.)
Use a profile generated with DEBUG=1 DESKTOP=0, and load app://sms.gaiamobile.org directly in a firefox nightly using this profile.

But now that we know what the root cause is, we can probably take it from here if you prefer :)
(In reply to Julien Wajsberg [:julienw] from comment #13)
> Use a profile generated with DEBUG=1 DESKTOP=0, and load
> app://sms.gaiamobile.org directly in a firefox nightly using this profile.

Whoah, that is snazzy!

With that, I can verify that adding "min-height:0" on #composer-messages-container fixes this.

Alternately (and perhaps more-correctly), you can move "overflow:auto" from #messages-container and up to #composer-messages-container.  I'd probably lean towards that solution, myself, as long as it doesn't break anything.

(Does #messages-container actually add any value? Perhaps that element should just be removed.)

(As noted in the spec[1], flex items only get special min-sizing behavior if they have overflow:visible (the default).  And in this case, we've currently got a flex item that does have overflow:visible, though it has a child with "overflow:auto; height:100%".  If we bump that overflow:auto up to the flex item (#composer-messages-container), we'll be OK.)

[1] http://dev.w3.org/csswg/css-flexbox/#min-size-auto
Flags: needinfo?(dholbert)
Attached patch fix v1Splinter Review
This fixes the problem for me, and per comment 14, I like this a bit better than relying on "min-height:0". (since it more semantically declares that this flex item is allowed to overflow, which tells the container not to bother imposing a special minimum size on it)
Attached file fix v1 as pull request
Here's a pull request of my attached fix. (Not sure if you're the right person to ask for review; feel free to punt to someone else if not. :))
Assignee: nobody → dholbert
Status: NEW → ASSIGNED
Attachment #8461833 - Flags: review?(felash)
Comment on attachment 8461833 [details] [review]
fix v1 as pull request

I'm one of the right persons but I redirect to Oleg because I'm busy with other blockers these days.

Thanks a lot !
Attachment #8461833 - Flags: review?(felash) → review?(azasypkin)
This makes the phone unusable for dogfooding.
Severity: normal → blocker
This needs automated testing.
Flags: in-testsuite?
Attachment #8461833 - Flags: review?(azasypkin) → review+
(I got some unrelated commits merged in when trying to edit the commit message in the pull request to note r=fabrice, so I just created a new pull request.)
Attachment #8462023 - Flags: review+
Attachment #8462023 - Attachment description: fix v1 pull request, with r=fabrice in commit message → fix v1 updated pull request, with r=fabrice in commit message
https://github.com/mozilla-b2g/gaia/commit/db4fe0b2b4d327e4e219cdebbf4f8377bdc3832e
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Julien, can we add a test that this can't regress again?
Flags: needinfo?(felash)
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
Duplicate of this bug: 1043809
See Also: → 1043518
Duplicate of this bug: 1043836
Reopening this bug since we tracked scroll position for "#messages-container" previously to lazily load messages by chunks. This patch removed "overflow: auto" from this element so scroll-dependent logic is broken now and user will see only first chunk of messages (10).

Probably SMS team can pick that bug from here if Daniel doesn't mind :)
Status: RESOLVED → REOPENED
Flags: needinfo?(dholbert)
Resolution: FIXED → ---
Oleg, can you take care to file a separate issue for adding integration tests and cover at least this use case?
Flags: needinfo?(felash) → needinfo?(azasypkin)
Blocks: 1043903
See Also: → 1043903
(In reply to Julien Wajsberg [:julienw] from comment #26)
> Oleg, can you take care to file a separate issue for adding integration
> tests and cover at least this use case?

Sure, filed a bug 1043903 for that.
Flags: needinfo?(azasypkin)
I'll take it as it's dogfooding blocker.
Assignee: dholbert → azasypkin
Hey Julien,

I followed Daniel's advice in comment 14 and removed redundant container. I've played with it in different cases and haven't noticed any other obvious regressions.
Attachment #8462564 - Flags: review?(felash)
Flags: needinfo?(dholbert)
Duplicate of this bug: 1043946
(In reply to Oleg Zasypkin [:azasypkin] from comment #25)
> Reopening this bug since we tracked scroll position for
> "#messages-container" previously to lazily load messages by chunks.
[...]
> Probably SMS team can pick that bug from here if Daniel doesn't mind :)

Ah, oops. Thanks!
Comment on attachment 8462564 [details] [review]
GitHub pull request URL

r=me
Attachment #8462564 - Flags: review?(felash) → review+
master: 77bda216fb4111d9ccfe5abbd26c97a0fd606f2c
Status: REOPENED → RESOLVED
Closed: 6 years ago6 years ago
Resolution: --- → FIXED
Target Milestone: --- → 2.1 S1 (1aug)
Whiteboard: [c=regression p= s= u=] → [c=regression p= s= u=][sms-sprint-2.1S1]
VERIFIED FIXED on v2.1.

Environmental Variables:
Device: Flame 2.1 KK (319 MB)
BuildID: 20141011000201 (full flash)
Gaia: f5d4ff60ffed8961f7d0380ada9d0facfdfd56b1
Gecko: d813d79d3eae
Gonk: 52c909e821d107d414f851e267dedcd7aae2cebf
Version: 34.0a2 (2.1)
Firmware: V180
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

The message thread scrolls as expected.
Status: RESOLVED → VERIFIED
QA Whiteboard: [QAnalyst-Triage+] → [QAnalyst-Triage?]
Flags: needinfo?(ktucker)
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
You need to log in before you can comment on or make changes to this bug.