Closed
Bug 1153808
Opened 9 years ago
Closed 9 years ago
[Messages] Thread is not marked as read (in UI only) when thread is opened from notification
Categories
(Firefox OS Graveyard :: Gaia::SMS, defect)
Tracking
(blocking-b2g:2.2+, b2g-v2.1 unaffected, b2g-v2.2 verified, b2g-master verified)
Tracking | Status | |
---|---|---|
b2g-v2.1 | --- | unaffected |
b2g-v2.2 | --- | verified |
b2g-master | --- | verified |
People
(Reporter: azasypkin, Assigned: azasypkin)
References
Details
(Keywords: regression, Whiteboard: [p=1])
Attachments
(2 files)
It happens only when Message app isn't run when user taps on notification (app was killed by LMK or device was restarted after receiving a message). STR: * Receive new message; * Open task manager and close Messages app; * Tap on notification and wait until Thread panel is opened; * Tap on back button and observe thread, message from the first step is belong to; Expected result: thread isn't marked as "unread"; Actual result: thread is marked as it still has unread messages; Internally thread is marked as read and everything will be displayed correctly when user restarts app. It likely came from the patch for bug 1112135, we try to mark-as-read thread node that hasn't been rendered yet.
Assignee | ||
Updated•9 years ago
|
Comment 1•9 years ago
|
||
Assignee | ||
Comment 2•9 years ago
|
||
Attached WIP patch (without tests) in case we need/want to fix it. I believe it will be almost "automatically" fixed with the future architecture approach when views are supposed to be very loosely coupled without such direct dependencies. So if it's not critical for v2.2, I'd rather avoid bringing any complexity here (though patch doesn't look risky), but let's ask for triage. Note for triagers: issue goes away if user restarts message app or exits and then enters thread once again.
blocking-b2g: --- → 2.2?
Comment 3•9 years ago
|
||
triage: regression and confusing users. please go ahead and fix on 2.2
blocking-b2g: 2.2? → 2.2+
Updated•9 years ago
|
Assignee: nobody → azasypkin
Assignee | ||
Updated•9 years ago
|
Status: NEW → ASSIGNED
Assignee | ||
Comment 4•9 years ago
|
||
Comment on attachment 8591730 [details] [review] [gaia] azasypkin:bug-1153808-when-thread-list-ui-ready > mozilla-b2g:master Hey Julien, I added unit tests + made some other tweaks in the initial patch. What do you think about it?
Attachment #8591730 -
Flags: review?(felash)
Comment 5•9 years ago
|
||
Comment on attachment 8591730 [details] [review] [gaia] azasypkin:bug-1153808-when-thread-list-ui-ready > mozilla-b2g:master I think we need the setTimeout but otherwise it works fine. Only think that doesn't work is: * launch the app (with a fair amount of threads) * enter the unread conversation asap * press back asap => the conversation is not marked as read before some time But I don't have a good idea that's also not complex..
Updated•9 years ago
|
Attachment #8591730 -
Flags: review?(felash)
Assignee | ||
Comment 6•9 years ago
|
||
Comment on attachment 8591730 [details] [review] [gaia] azasypkin:bug-1153808-when-thread-list-ui-ready > mozilla-b2g:master Hey Julien, > I think we need the setTimeout but otherwise it works fine. I've returned "setTimeout" back with comment that explains why we need it (in a separate commit). > Only think that doesn't work is: > * launch the app (with a fair amount of threads) > * enter the unread conversation asap > * press back asap > => the conversation is not marked as read before some time > > But I don't have a good idea that's also not complex.. Yeah, I can't find reasonably simple solution for this as well. We can revisit it later if it really annoys the user. Thanks!
Attachment #8591730 -
Flags: review?(felash)
Updated•9 years ago
|
Blocks: sms-sprint-2.2S11
Whiteboard: [p=1]
Comment 7•9 years ago
|
||
Comment on attachment 8591730 [details] [review] [gaia] azasypkin:bug-1153808-when-thread-list-ui-ready > mozilla-b2g:master r=me thanks ! Don't forget to squash :)
Attachment #8591730 -
Flags: review?(felash) → review+
Updated•9 years ago
|
Keywords: checkin-needed
Comment 9•9 years ago
|
||
Pull request has landed in master: https://github.com/mozilla-b2g/gaia/commit/a7e198fb0aa004c5eebdf7f00a4431538f54f87d
Updated•9 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 10•9 years ago
|
||
Comment on attachment 8591730 [details] [review] [gaia] azasypkin:bug-1153808-when-thread-list-ui-ready > mozilla-b2g:master [Approval Request Comment] [Bug caused by] (feature/regressing bug #): bug 1112135 [User impact] if declined: thread won't be marked as read, if user opens Messages from notification until user re-enters thread once again or restarts application. [Testing completed]: yes, manual and unit test [Risk to taking this patch] (and alternatives if risky): relatively low [String changes made]: n/a
Attachment #8591730 -
Flags: approval-gaia-v2.2?
Updated•9 years ago
|
Attachment #8591730 -
Flags: approval-gaia-v2.2? → approval-gaia-v2.2+
Comment 11•9 years ago
|
||
v2.2: https://github.com/mozilla-b2g/gaia/commit/f06ea5c7ca524f7e42596893c71e0fac36db923f
Target Milestone: --- → 2.2 S11 (1may)
Assignee | ||
Comment 12•9 years ago
|
||
Ooops, I see that patch applied to v2.2 without conflicts, but it uses method that doesn't exist in v2.2, so so some Messages integration tests are perma-red (see bug 1148152) - I'll prepare separate patch for v2.2. Ryan, could you please backout this patch from v2.2 for now? Sorry for the inconvenience. Thanks!
Flags: needinfo?(ryanvm)
Comment 13•9 years ago
|
||
v2.2: https://github.com/mozilla-b2g/gaia/commit/265ca0bc9408c21fc4b25a259fcee7fb642cd06b
Flags: needinfo?(ryanvm)
Comment 14•9 years ago
|
||
Assignee | ||
Comment 15•9 years ago
|
||
Comment on attachment 8597414 [details] [review] [gaia] azasypkin:v2.2-bug-1153808-when-thread-list-ui-ready > mozilla-b2g:v2.2 Hey Steve, Since Julien is on PTO on Monday, could you please help to review this v2.2-only patch? I added changes needed for v2.2 as separate commit so that you can see the difference. Thanks!
Attachment #8597414 -
Flags: review?(schung)
Comment 16•9 years ago
|
||
Comment on attachment 8597414 [details] [review] [gaia] azasypkin:v2.2-bug-1153808-when-thread-list-ui-ready > mozilla-b2g:v2.2 LGTM, thanks!
Attachment #8597414 -
Flags: review?(schung) → review+
Comment 17•9 years ago
|
||
v2.2: https://github.com/mozilla-b2g/gaia/commit/efd66b5599e058fb8456350033e4c292b56fd0c2
Comment 18•9 years ago
|
||
This issue is verified fixed on Flame 3.0 and 2.2. Message thread is now correctly being marked as read after following STR. Device: Flame 3.0 BuildID: 20150428010206 Gaia: 0636405f0844bf32451a375b2d61a2b16fe33348 Gecko: caf25344f73e Gonk: b83fc73de7b64594cd74b33e498bf08332b5d87b Version: 40.0a1 (3.0) Firmware Version: v18D-1 User Agent: Mozilla/5.0 (Mobile; rv:40.0) Gecko/40.0 Firefox/40.0 Device: Flame 2.2 BuildID: 20150428002500 Gaia: 9f6b1b9082662ba2c14168fc66bb02b4df3141e5 Gecko: e79c19bf19bf Gonk: ebad7da532429a6f5efadc00bf6ad8a41288a429 Version: 37.0 (2.2) Firmware Version: v18D-1 User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0
Status: RESOLVED → VERIFIED
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(ktucker)
Updated•9 years ago
|
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
Assignee | ||
Updated•9 years ago
|
Whiteboard: [p=1] → [p=1][sms-sprint-2.2S11]
Updated•9 years ago
|
Whiteboard: [p=1][sms-sprint-2.2S11] → [p=1]
You need to log in
before you can comment on or make changes to this bug.
Description
•