Closed Bug 1206678 Opened 4 years ago Closed 4 years ago

Outgoing MMS contains subject that was typed by user, but then hidden

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(blocking-b2g:2.5+, b2g-v2.2 affected, b2g-v2.2r affected, b2g-master verified)

VERIFIED FIXED
FxOS-S8 (02Oct)
blocking-b2g 2.5+
Tracking Status
b2g-v2.2 --- affected
b2g-v2.2r --- affected
b2g-master --- verified

People

(Reporter: azasypkin, Assigned: steveck)

References

Details

(Keywords: regression, Whiteboard: [p=1])

Attachments

(2 files)

[Blocking Requested - why for this release]: old regression, we can send the data (MMS subject) user doesn't want to send. Happens only when user sends MMS (if finally message becomes SMS, subject won't be sent). The fix is supposed to be easy and safe.

STR:
* Open Messages app and enter new message composer;
* Add a subject and type some text in subject field;
* Add an attachment;
* Hide/remove subject using the top right menu;
* Send the message.

Expected result: message is sent with attachment, but without subject;

Actual result: message is sent with both attachment and hidden subject.

It's year old regression from bug 1067228.
blocking-b2g: 2.5? → 2.2?
Asking for 2.2+ status because the regression is in 2.2 and the patch should be easy. It's quite an edge case but I can feel a user could encounter it.
[Blocking Requested - why for this release]:
blocking-b2g: 2.2? → 2.5?
blocking-b2g: 2.5? → 2.5+
Whiteboard: [p=1]
Assignee: nobody → schung
Hi Morpheus,
The remove subject option seems not clear here because the current behavior of the remove subject is  only "hide" the subject actually, so the subject will display with previous text content when user chooses to add the subject again. Should we remove the text when user click "remove subject", or simply keep the current behavior?
Flags: needinfo?(mochen)
In bug 1176976, we also were considering the simplest solution when "Compose.getSubject" just doesn't return anything if it's not visible [1], so the current behavior is kept, but bugs are fixed :)

[1] https://github.com/mozilla-b2g/gaia/pull/30898#discussion_r39918179
(In reply to Oleg Zasypkin [:azasypkin][⏰UTC+1] from comment #4)
> In bug 1176976, we also were considering the simplest solution when
> "Compose.getSubject" just doesn't return anything if it's not visible [1],
> so the current behavior is kept, but bugs are fixed :)
> 
> [1] https://github.com/mozilla-b2g/gaia/pull/30898#discussion_r39918179

Yeah, it's also an option in my mind, but just want to confirm how we handle the subject when user choose remove the subject.
Please refer to [1] for all the flows related to Subject we use currently.

[1] https://wiki.mozilla.org/images/3/39/FFOS_MessageApp_V1.3_20131125_V8.0.pdf

Also I know we don't want to clear the text because we want the text to be shown again when the user selects "show subject" again (this is not in the UX spec but was discussed on a bug -- I could find it if you really want to but this would be difficult :p).
There are other solutions to do that, but the simplest is to not clear :)

That's why the best and only solution is what we did before bug 1067228: retrieving subject's value should check the subject visibility.
Target Milestone: --- → FxOS-S8 (02Oct)
Hey Steve, I think we can move forward to recover the previous behavior even if Morpheus doesn't answer.
Sorry for my late reply. I just get back from business trip, and thank for waiting.

IMO, the added subject should be entirely removed since the term says "remove subject". I understand the reason to just hide the subject is to prevent user retyping again, however, most use cases of subject are pretty short, especially in Message, it might be unnecessary to provide the hidden feature. Moreover, the hidden feature can also be confusing when user choose "add subject". So, I would prefer to entirely remove the subject.
Flags: needinfo?(mochen)
Thanks for all the reply, so 2.5 I think we can simply keep the original behavior as Julien said in comment 6. Since UX has some concerns about the hide/remove subject, I'll also create another bug for subject removal as backlog.
We can discuss a little more on the additional bug.
Comment on attachment 8667782 [details] [review]
[gaia] steveck-chung:bug-1206678-subject-hidden > mozilla-b2g:master

Hi Oleg, this patch is simply align with the draft saving changes. I think the handling here should be the same?
Attachment #8667782 - Flags: review?(azasypkin)
(In reply to Julien Wajsberg [:julienw] (away Oct 1st, 2nd) from comment #10)
> We can discuss a little more on the additional bug.

Bug 1209895 created for the subject removal.
Comment on attachment 8667782 [details] [review]
[gaia] steveck-chung:bug-1206678-subject-hidden > mozilla-b2g:master

As we all agreed, let's move change to "Compose.getSubject()" instead (and remove workaround I've made in [1]), please set r? again whenever you're ready :)

Thanks!

[1] https://github.com/mozilla-b2g/gaia/blob/2433ba5a10ede2f3f221376a4ed7fc3c33ad266d/apps/sms/views/conversation/js/conversation.js#L2914
Attachment #8667782 - Flags: review?(azasypkin)
Comment on attachment 8667782 [details] [review]
[gaia] steveck-chung:bug-1206678-subject-hidden > mozilla-b2g:master

Patch updated for the getSubject part.
Attachment #8667782 - Flags: review?(azasypkin)
Comment on attachment 8667782 [details] [review]
[gaia] steveck-chung:bug-1206678-subject-hidden > mozilla-b2g:master

Looks good, thanks for the quick fix!
Attachment #8667782 - Flags: review?(azasypkin) → review+
Thanks!

In master: https://github.com/mozilla-b2g/gaia/commit/715aa5454e3f034dc7a340430411445ccf5bf4d4
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
According to the STR of Comment 0, this bug has been verified as pass on latest Flame KK v2.5 and Aries KK v2.5.
Actual results: Message is sent with attachments, but without subject.
See attachment: verified_Aries KK v2.5.3gp
Reproduce rate: 0/6

Device: Flame KK v2.5 (Pass)
Build ID               20151008150210
Gaia Revision          e698df503ff700eb5782e3d50c6eb753567d3451
Gaia Date              2015-10-08 17:26:52
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/e5f1bc63ad52d0eb86f7fb838226ca6036774660
Gecko Version          44.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20151008.182756
Firmware Date          Thu Oct  8 18:28:08 EDT 2015
Firmware Version       v18D v4
Bootloader             L1TC000118D0

Device: Aries KK v2.5 (Pass)
Build ID               20151008233852
Gaia Revision          e698df503ff700eb5782e3d50c6eb753567d3451
Gaia Date              2015-10-08 17:26:52
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/23b7f289df923c01e692299fcd4be7029de8b155
Gecko Version          44.0a1
Device Name            aries
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.worker.20151008.225742
Firmware Date          Thu Oct  8 22:57:51 UTC 2015
Bootloader             s1
Status: RESOLVED → VERIFIED
QA Whiteboard: [MGSEI-Triage+]
You need to log in before you can comment on or make changes to this bug.