[Messages] when we're in an activity and we delete all messages in the thread, we should close the activity

VERIFIED FIXED in Firefox OS v2.0

Status

VERIFIED FIXED
4 years ago
4 years ago

People

(Reporter: ankit93040, Assigned: julienw, Mentored)

Tracking

unspecified
2.1 S5 (26sep)

Firefox Tracking Flags

(blocking-b2g:2.0+, b2g-v2.0 verified, b2g-v2.1 verified, b2g-v2.2 verified)

Details

(Whiteboard: [g+][LibGLA, Dev, B][sms-papercuts][sms-sprint-2.1S5])

Attachments

(3 attachments)

(Reporter)

Description

4 years ago
User Agent: Mozilla/5.0 (Windows NT 6.1) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/36.0.1985.125 Safari/537.36

Steps to reproduce:

1. go to share
2. share an image with message
3. send it.
4. options-delete message->select all->delete->delete
5. Remains in message app


Actual results:

Remains in message app


Expected results:

No way to go back to gallery app
(Reporter)

Updated

4 years ago
Flags: needinfo?(felash)
Whiteboard: [g+][LibGLA, Dev, B]
(Assignee)

Comment 1

4 years ago
[Blocking Requested - why for this release]: regression, I think we should block for this.
Blocks: 936729
blocking-b2g: --- → 2.0?
Flags: needinfo?(felash)
Keywords: regression
Hey Jenny,

We need your advice on this issue :)

What is expected result for the STR in comment 0? The easiest option would be to just leave activity (close SMS app) and return to the gallery app.

We're also have another similar case, if user opens SMS app as inline activity and then sends SMS to _several_ recipients, user is redirected to the thread list panel (inbox), but after ~3 sec we're automatically leave activity and return user to the original app. I don't know about rationale behind that, but I'm sure Julien knows :)

Thanks!
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(jelee)
Flags: needinfo?(felash)
(Assignee)

Comment 3

4 years ago
(In reply to Oleg Zasypkin [:azasypkin] from comment #2)
> Hey Jenny,
> 
> We need your advice on this issue :)
> 
> What is expected result for the STR in comment 0? The easiest option would
> be to just leave activity (close SMS app) and return to the gallery app.
> 
> We're also have another similar case, if user opens SMS app as inline
> activity and then sends SMS to _several_ recipients, user is redirected to
> the thread list panel (inbox), but after ~3 sec we're automatically leave
> activity and return user to the original app. I don't know about rationale
> behind that, but I'm sure Julien knows :)

Yeah, the idea is to _see_ that the messages are correctly sent (that's why we go to the thread list as we do when we're not in an activity), but we're still leaving the activity because there is nothing left to be done.

I'm sure you can find the original bug if you blame the code :)
Flags: needinfo?(felash)
(Assignee)

Comment 4

4 years ago
(In reply to Julien Wajsberg [:julienw] from comment #1)
> [Blocking Requested - why for this release]: regression, I think we should
> block for this.

Actually I'm not so sure it's a regression: previously to bug 936729 (so in versions before v2.0) we could not go back to the gallery activity either. So maybe we should not block on this after all, because it's an edge case and not a regression.
Keywords: regression
Julien, so does that mean we regressed this functionality after bug 936729 landed? Do you know what caused it?

Regarding the blocking question - Julien is correct that this is not a regression from 1.3, so shouldn't block. The only reason we could block on this is if the product owners decided that this new feature is so important we could not ship without it.
blocking-b2g: 2.0? → backlog
Flags: needinfo?(felash)
(Assignee)

Comment 6

4 years ago
Dietrich: no, it's more that this was overlooked when we landed bug 936729.
Flags: needinfo?(felash)

Comment 7

4 years ago
(In reply to Oleg Zasypkin [:azasypkin] from comment #2)
> Hey Jenny,
> 
> We need your advice on this issue :)
> 
> What is expected result for the STR in comment 0? The easiest option would
> be to just leave activity (close SMS app) and return to the gallery app.
> 
> We're also have another similar case, if user opens SMS app as inline
> activity and then sends SMS to _several_ recipients, user is redirected to
> the thread list panel (inbox), but after ~3 sec we're automatically leave
> activity and return user to the original app. I don't know about rationale
> behind that, but I'm sure Julien knows :)
> 
> Thanks!

Hi Oleg,

In this case, it makes more sense to return to the original app that initiates the inline SMS activity. So let's close the activity. Same should happen when user sends message from contact and deletes all the messages :) Thanks!
Flags: needinfo?(jelee)
(Reporter)

Comment 8

4 years ago
Hi Wayne

Greetings!

Will you please request someone to look into this.
I request you to assign this to someone.

Thanks.

Ankit
Flags: needinfo?(wchang)
Ankit -

This has been backlogged as commented above and will be addressed by relevant teams.
Flags: needinfo?(wchang)
(Assignee)

Updated

4 years ago
Whiteboard: [g+][LibGLA, Dev, B] → [g+][LibGLA, Dev, B][sms-papercuts]
(Assignee)

Updated

4 years ago
Summary: [sms]share activity is not proper → [Messages] when we're in an activity and we delete all messages in the thread, we should close the activity
(Assignee)

Comment 10

4 years ago
Adding this bug as mentored bug. The relevant code is in thread_ui.js.

To fix this bug it's better to use a device, but it's probably possible to fix it using B2G Desktop.
Mentor: felash
Whiteboard: [g+][LibGLA, Dev, B][sms-papercuts] → [g+][LibGLA, Dev, B][sms-papercuts][lang=js][good first bug]
(Assignee)

Updated

4 years ago
Duplicate of this bug: 1052445
Hi Julien,thanks for your comments.
Since partner is requesting this, Do you think if it's feasible we could have this fix on 2.0?
and do you mind providing the fix for us?
Thanks very much.
Flags: needinfo?(felash)
(Assignee)

Comment 13

4 years ago
Rachelle, I'm asking a blocker status here.

I personally don't think it's _that_ important to need an urgent fix. If partner want it it's easy to provide by themselves.

If this bug is made a blocker we can fix it quickly.
blocking-b2g: backlog → 2.0?
Flags: needinfo?(felash)
triage: not blocking bug and corner case.
blocking-b2g: 2.0? → backlog

Comment 15

4 years ago
Dear Julien,

I think that it can be a blocking issue for us. 
If you share a PR for this issue, we can test it and let you know result.
Please let me know your opinion.

Thank you.
Flags: needinfo?(felash)
(Assignee)

Comment 16

4 years ago
Dear Youngjun Kim,

The issue has not been prioritized on our side, therefore we need to work on issues that have an higher priority. If you think this is a blocking issue for you, please raise the problem with your Mozilla contact.
Flags: needinfo?(felash)
[Blocking Requested - why for this release]:

As mentioned in Comment#15, this is a IOT blocker for partner, please help to prioritize this issue, if it is too late for 2.0, at least we should nominate as 2.1 blocker and fix ASAP

Thanks

Vance
blocking-b2g: backlog → 2.0?
Comms triage does not consider it as a blocker, so we let Release management the decision to block it or not.
Flags: needinfo?(bbajaj)
Flame is also reproducible.Triage to set it to 2.0+.
blocking-b2g: 2.0? → 2.0+
(Assignee)

Comment 20

4 years ago
Will handle this.
Assignee: nobody → felash
(Assignee)

Comment 21

4 years ago
Created attachment 8493691 [details] [review]
github PR

I was surprised that the new onHeaderAction didn't have any unit test, but well, here are at least some tests that use it.

This will need a branch patch for v2.0 and maybe v2.1.
Attachment #8493691 - Flags: review?(azasypkin)
(Assignee)

Comment 22

4 years ago
Created attachment 8493698 [details] [review]
github PR for v2.0
(Assignee)

Comment 23

4 years ago
master patch applies cleanly for v2.1, and v2.0 only needs a minor change, so I won't request review again on the patch for v2.0.
(Assignee)

Comment 24

4 years ago
Tested my patches on v2.0 and master on Flame.
(Assignee)

Updated

4 years ago
Whiteboard: [g+][LibGLA, Dev, B][sms-papercuts][lang=js][good first bug] → [g+][LibGLA, Dev, B][sms-papercuts][sms-sprint-2.1S5]
Target Milestone: --- → 2.1 S5 (26sep)
Comment on attachment 8493691 [details] [review]
github PR

r=me, just tiny nit on GitHub.

Thanks!
Attachment #8493691 - Flags: review?(azasypkin) → review+
(Assignee)

Comment 26

4 years ago
master: f8d810cd48db8886460bb931e220892f19c2dd23
Status: NEW → RESOLVED
Last Resolved: 4 years ago
status-b2g-v2.2: --- → fixed
Resolution: --- → FIXED
(Assignee)

Comment 27

4 years ago
Comment on attachment 8493691 [details] [review]
github PR

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): bug 936729
[User impact] if declined: user can stay stuck in an activity (but maybe the "home" button can escape the activity still)
[Testing completed]: yes, unit tests and manually
[Risk to taking this patch] (and alternatives if risky): low
[String changes made]: none
Attachment #8493691 - Flags: approval-gaia-v2.1?
Attachment #8493691 - Flags: approval-gaia-v2.0?

Updated

4 years ago
Flags: needinfo?(bbajaj)
Comment on attachment 8493691 [details] [review]
github PR

Adding verifyme to get help with verification here once it lands..
Attachment #8493691 - Flags: approval-gaia-v2.1?
Attachment #8493691 - Flags: approval-gaia-v2.1+
Attachment #8493691 - Flags: approval-gaia-v2.0?
Attachment #8493691 - Flags: approval-gaia-v2.0+

Updated

4 years ago
Keywords: verifyme
(Reporter)

Comment 30

4 years ago
Hi Julienw

Precondition:- https://github.com/mozilla-b2g/gaia/pull/23901 is merged & then if i merged this bug pull request then I have one issue.

Data shared from other applications are not shown as Drafts if it has only recipient & no data. I mean go to any saved contact in contact app & send a message from there, don't write anything in messgae click(x) & save as draft. It will not be saved as draft if it has only recipient.

Please, Julienw any idea why this issue is happening. Same has happen with a few more already discussed with you..

Need your little time to investigate on this.

Thanks a lot.
Flags: needinfo?(felash)
(Assignee)

Comment 31

4 years ago
Ankit, this is not related to this bug. You should know how we work with bugzilla now, please file a separate bug!

Thanks
Flags: needinfo?(felash)

Comment 32

4 years ago
Verified it on v2.0
It returns to the original app.

* Build information:
 - Gaia      279c5ee3a2b4cfd1484196a409e05ef579de1b53
 - Gecko     https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/f5655da30c8b
 - BuildID   20141002160200
 - Version   32.0
 - ro.build.version.incremental=27
 - ro.build.date=Thu Sep  4 14:59:02 CST 2014
Status: RESOLVED → VERIFIED
Keywords: verifyme

Comment 33

4 years ago
This issue has been verified successfully on Flame2.0&2.1&2.2
Verify video:"verify_1052447.mp4".
**From comment32, Flame2.0 is OK
User Agent:
Flame2.1: Mozilla/5.0(Mobile;rv:34.0) Gecko/34.0 Firefox/34.0
Flame2.2: Mozilla/5.0(Mobile;rv:37.0) Gecko/37.0 Firefox/37.0

Flame2.1 build:
Gaia-Rev        dbaf3e31c9ba9c3436e074381744f2971e15c7bf
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/ebce587d2194
Build-ID        20141203001205
Version         34.0
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20141203.034907
FW-Date         Wed Dec  3 03:49:18 EST 2014
Bootloader      L1TC00011880

Flame2.2 bulid:
Gaia-Rev        725685831f5336cf007e36d9a812aad689604695
Gecko-Rev       https://hg.mozilla.org/mozilla-central/rev/2c9781c3e9b5
Build-ID        20141203040207
Version         37.0a1
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20141203.072513
FW-Date         Wed Dec  3 07:25:25 EST 2014
Bootloader      L1TC00011880
status-b2g-v2.0: fixed → verified
status-b2g-v2.1: fixed → verified
status-b2g-v2.2: fixed → verified

Comment 34

4 years ago
Created attachment 8531843 [details]
Verified video: verify_1052447.MP4
You need to log in before you can comment on or make changes to this bug.