Closed Bug 948958 Opened 11 years ago Closed 11 years ago

[Messages][MMS] MMS does not display correctly when it has a subject but no content.

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(blocking-b2g:-, b2g-v1.2 wontfix, b2g-v1.3 fixed, b2g-v1.4 fixed)

VERIFIED FIXED
1.3 C3/1.4 S3(31jan)
blocking-b2g -
Tracking Status
b2g-v1.2 --- wontfix
b2g-v1.3 --- fixed
b2g-v1.4 --- fixed

People

(Reporter: lolimartinezcr, Assigned: steveck)

Details

Attachments

(7 files)

Attached image 2013-12-11-16-12-29.png (deleted) —
tested in: 1.3 12/11 Gecko 3b2d739 Gaia cbd2921 Steps: 1. Tap messages aplication. 2. Tap in new message icon. 3. Tap in top-right icon and select "Add subject" option. 4. Write phone number, introduce only spaces in subject and body is empty. 5. Press "Send" button (See attached) Actual result: "There is a problem with the attached file and it cannot be downloaded." (see attached)
blocking-b2g: --- → 1.3?
Attached image 2013-12-11-16-12-40.png (deleted) —
So probably Gecko does not like empty subject. Or maybe we're trimming it? Ayman, should we accept sending a subject with only spaces? Or should we handle this just like an empty subject (and it would mean send the message as a SMS if it contains only text)?
Flags: needinfo?(aymanmaat)
Only operation done to subject is exchanging multiple spaces and breaklines into single space [dom.subject.value.replace(/\n\s*/g, ' ')]
I tested it myself, and the issue is happening also with any subject. The issue is that the message is converted to MMS as soon as there is a subject, but ThreadUI.buildMessageDOM assumes that if the message is a MMS it must have attachments, and that if it has no attachment it's an error. See [1] and [2]. The issue is happening in the same way when _receiving_ a MMS with a subject and no attachment. And what's worse is that it happens in 1.1 and 1.2 as well! Therefore I'm gonna ask koi+ on this. [1] https://github.com/mozilla-b2g/gaia/blob/master/apps/sms/js/thread_ui.js#L1397-L1398 [2] https://github.com/mozilla-b2g/gaia/blob/master/apps/sms/js/thread_ui.js#L1428
blocking-b2g: 1.3? → koi?
Flags: needinfo?(aymanmaat)
Summary: [Messages][MMS] MMS isn't sent when body is empty and subject only has spaces → [Messages][MMS] MMS does not display correctly when it has a subject but no attachment.
Correction: it works fine both in 1.1, 1.2 and 1.3 as soon as there is a body content (text or attachment). So it's maybe not enough a blocker to warrant koi+, but we can fix it for 1.3+ for sure.
Summary: [Messages][MMS] MMS does not display correctly when it has a subject but no attachment. → [Messages][MMS] MMS does not display correctly when it has a subject but no content.
Does this reproduce on 1.1? Triage Notes: Probably not worth blocking on. It doesn't feel critical enough to block 1.2 at this point.
Keywords: qawanted
QA Contact: jzimbrick
As far as I can tell, there does not appear to be a way to add a subject line to SMS/MMS when using the latest 1.1 or 1.2 Mozilla builds on a Buri device. I have the option in 1.1 to select attachments from Video, Wallpaper, Music, Gallery, and Camera, but see nothing concerning a subject line. In 1.2 only Video, Music, Gallery, and Camera are shown. Sending an SMS in 1.1 and 1.2 that is filled only with spaces simply sends an empty message to the receiving phone, no error message is displayed. Is there a way to add a subject line to an SMS message in the 1.1 and 1.2 branches that I'm unaware of? Environmental Variables: Device: Buri v1.1 Mozilla RIL BuildID: 20131213041208 Gaia: 6ff3a607f873320d00cb036fa76117f6fadd010f Gecko: bdac595a4e46 Version: 18.0 Base Image: V1.2_20131115 Device: Buri v1.2 Mozilla RIL BuildID: 20131213004002 Gaia: 1aca7c4860e39b1a9969807d335dcf9f070ea9b3 Gecko: a2b69b561d9b Version: 26.0 Base Image: V1.2_20131115
Keywords: qawanted
You can't send a message with a subject but you can receive one. I actually tried it myself on my 1.1 device when I wrote comment 4 so the only qawanted thing is with 1.2. Also, I'd like to know what's the behavior on Android, but I don't know if that's the goal of qawanted. Thanks!
Keywords: qawanted
Thanks for the clarification, Julien. Just tested this on the latest Buri 1.2 Mozilla build. When receiving a message with a subject line but no body text from a another device running the latest 1.4 Buri Mozilla build, an error is displayed stating: "There is a problem with the attached file and it cannot be downloaded." Environmental Variables: Device: Buri v1.2 Mozilla RIL BuildID: 20131216004002 Gaia: fcf1c2fe020c29da4755621cbffdc1a333a43be9 Gecko: 129ad3c335a5 Version: 26.0 Base Image: V1.2_20131115 I'll leave the qawanted tag as I do not have an Android phone available.
Checked with two testers here that did have personal Android devices. There didn't seem to be any option for them to send an SMS with a subject from their devices, but I did send an SMS with a Subject and a blank body from a Buri device running the latest 1.4 Buri Mozilla build. On the first Android (Galaxy S3 running 4.1 Jellybean), the message was received without error, displaying the subject line text and stating that the body was empty. On the second Android (LGG 2 running 4.2.2 Jellybean), the message with a subject line but no body simply wasn't received at all. Sending an SMS with text was received fine and sending an MMS with picture attachment was received fine. And, though not asked for, I checked how this works on iPhone 5 running iOS version 7.0.3. The device isn't able to send a message that only has a subject and no body, the send button does not function until body text is entered. When receiving a message with a subject but no body text, the message is received without error and shows the subject text in bold and a blank body. Hopefully this was the sort of information you were looking for, removing the qawanted tag for now.
Keywords: qawanted
Yes, this is very comprehensive, thanks a lot! Ayman, how should we display a MMS that has a subject but no content? And please also answer for older versions that don't show a subject too, as we might need to do a patch for this as well. ;)
Flags: needinfo?(aymanmaat)
I cannot tell from the two attached screenshots, but just to be clear: the user can still see the body of the message if there is a message body, yes? Just not the attachment? The context in which the error message screenshot (attached to this bug) appears is not clear. Thanks!
The error message appears as soon as there is a subject but no content at all (it means: no attachment, no text). As soon as there is some content, there is no error.
(In reply to Stephany Wilkes from comment #12) > I cannot tell from the two attached screenshots, but just to be clear: the > user can still see the body of the message if there is a message body, yes? > Just not the attachment? As Julien has said the failure is happening when sending a MMS without body content STR: 1. Tap messages aplication. 2. Tap in new message icon. 3. Tap in top-right icon and select "Add subject" option. 4. Write phone number, introduce any subject and body is empty. 5. Press "Send" button Actual result: It appears the subject of the message and instead of a empty body the message "There is a problem with the attached file and it cannot be downloaded." see screenshot
Attached image 2013-12-18-12-20-32.png (deleted) —
Bumping to 1.3 in triage. It's possible that the impact of landing a patch this late is bigger than the benefit of fixing this feature now.
blocking-b2g: koi? → 1.3?
triage: 1.3+. broken new feature implementation
blocking-b2g: 1.3? → 1.3+
I can take a look first, but I need a clear answer for solving the problem. For 1.3, how about we just showing a single empty line in the content field with subjuct in the message bubble? For the older version, we don't handle the subject. Maybe we can show the subject in the content field if there is only subject but no message content in this case?
Assignee: nobody → schung
I think we need a message like "Empty message" inside the bubble, but written with a different font style than usual messages. We don't need to care about older versions now, since koi+ was rejected ;) This is quite an edge case anyway.
Target Milestone: --- → 1.3 C2/1.4 S2(17jan)
(In reply to Julien Wajsberg [:julienw] from comment #19) > I think we need a message like "Empty message" inside the bubble, but > written with a different font style than usual messages. > > We don't need to care about older versions now, since koi+ was rejected ;) > This is quite an edge case anyway. on yes you're right, since it's 1.3 blocking issue now, we don't have to take care about the old version case.
Target Milestone: 1.3 C2/1.4 S2(17jan) → ---
Target Milestone: --- → 1.3 C2/1.4 S2(17jan)
By the way, I'm ok with the "Empty message" inside the bubble. We just need UX input for some confirmation. Hi Ayman, if we have "Empty message" inside the bubble, do we still need the message in attachment 8345909 [details] for no attachment error case? If we both have 'Empty message' and no attachment error cases, do we need to apply different font style for both case?
Attached file Link to github (deleted) —
Hi Borja, I commit this patch based on Julien's previous comment, since he might busy on other tasks, could you please take a look first? Thanks.
Attachment #8361485 - Flags: review?(borja.bugzilla)
FYI I just pinged Ayman by email so that he can have a quick look here. Thanks to both of you :)
Attached image Spinner.png (deleted) —
Testing the code I've just realized that the spinner is not hidden when sending this type of MMS with Subject but empty body and no attachments. Steve, could you take a look? Thanks!
Could this be bug 957084 ?
(In reply to Borja Salguero [:borjasalguero] from comment #24) > Created attachment 8361568 [details] > Spinner.png > > Testing the code I've just realized that the spinner is not hidden when > sending this type of MMS with Subject but empty body and no attachments. > Steve, could you take a look? Thanks! Might be the problem in bug 957084 like Julien said, but I can send the subject only message on my device. If we are actually sending the empty message(it's still a valid mms), so showing the spinner while sending is quite normal, isn't it? :)
Attached image Screen Shot 2014-01-06 at 17.46.41.png (deleted) —
Hi Ayman, the below 2 message bubble is the result after applying the patch. The content shows 'empty message' is the valid mms message with subject only. It's Julien's opinion that showing 'empty message' wording inside the bubble with differnt font type and color. The bottom one is a mms message without subject and attachment, which should be a rare case and hardly reproducible. We will display a warning message with error icon in the bubble. It would be great if you have better solution for displaying these 2 different cases, thanks.
Attachment #8361576 - Flags: feedback?(aymanmaat)
referencing comment 27 passing ni? to Jose as this is more of a visual design question.
Flags: needinfo?(aymanmaat) → needinfo?(vittone)
Target Milestone: 1.3 C2/1.4 S2(17jan) → 1.3 C3/1.4 S3(31jan)
Passing ni to Ayman again since he has more info about the original spec.
Flags: needinfo?(vittone) → needinfo?(aymanmaat)
Hi Ayman, I think this would be more likely an UX issue first, we need a clear answer for empty message(subject only) MMS about how we display the message in the bubble: 1) My idea is we can simply showing the bubble with empty content. The message content in bubble should have min height for sending cursor. 2) Julien's idea is showing the 'empty message' in the message content with different style. Jose can provide some feedback if we choose this solution. 3) Any other way to display the empty message?
Target Milestone: 1.3 C3/1.4 S3(31jan) → 1.3 C2/1.4 S2(17jan)
Target Milestone: 1.3 C2/1.4 S2(17jan) → 1.3 C3/1.4 S3(31jan)
(In reply to Steve Chung [:steveck] from comment #30) Hi Steve, > 1) My idea is we can simply showing the bubble with empty content. The > message content in bubble should have min height for sending cursor. Seems to be enough to say that something is empty, it's explained via its shape. I understand Julien's idea on being clear, but finally the solution gets so complicated. So, from the UX PoV your first idea is just fine.
Flags: needinfo?(aymanmaat)
Sorry José, I don't understand why it is so "complicated" to show a message with a different style?
(In reply to Julien Wajsberg [:julienw] from comment #32) > Sorry José, I don't understand why it is so "complicated" to show a message > with a different style? Hi Julien, I would say that: - It's not about the complexity of showing a message with a different style, is about how to communicate an empty message. - Having a placeholder text like that will take the focus off the actual user input, in this case the subject. We should always highlight the content rather than the chrome. - An empty message explains what's on it. Adding an explanatory text is redundant and also wierd because that space is reserved to be filled with other person words. - This kind of message could work on the preview of the message, on the messages list, before entering the thread. Like Apple's email does, they don't modify the message itself. - The time stamp is going to be moved inside the bubble as we defined here https://bugzilla.mozilla.org/show_bug.cgi?id=871432 will also balance that emptiness.
Oki, makes sense, thanks!
Hi Borja, I update the patch based on José's comment in comment 33. Please review the patch for the final result, thanks!
We punted this from 1.2. Why is this necessary for 1.3? Renoming for more discussion.
blocking-b2g: 1.3+ → 1.3?
I think we didn't block from 1.2 mainly because the bug was found too late. I don't think this is too late for 1.3 especially if we don't add new strings.
(In reply to Julien Wajsberg [:julienw] from comment #37) > I think we didn't block from 1.2 mainly because the bug was found too late. > > I don't think this is too late for 1.3 especially if we don't add new > strings. Well right, but that would mean that we should be pushing for approval here, not blocking.
Already in 1.2, would like to wait till 1.4 for a fix. Unblocking for 1.3 now.
blocking-b2g: 1.3? → -
Attachment #8361485 - Flags: review?(borja.bugzilla) → review+
don't understand why bugzilla tells me i have a ni? for this bug when the bug tells me I don't. Great to see this in V1.3
> Great to see this in V1.3 Won't be with the current flags, but I think we can request an approval to Fabrice as the code looks fairly safe. I haven't tested it myself so maybe Borja can do the request if he tested it and checked that it worked ?
This behaviour is improved with the visual refresh, so I would like to check tomorrow with UX if we can move this to 1.4 for getting the whole solution. I'll come back tomorrow with more info about this! Thanks Julien for raising this.
But if we can get this patch into 1.3 then I think we should, instead of waiting for doing something better in 1.4.
Attached image Subject but empty body mms (deleted) —
Jose, this is the result when the patch is applied. Could you take a look? We would like to know if we should push for adding this to 1.3, or wait the final solution coming with the refresh in 1.4. Thanks!
Attachment #8369910 - Flags: review?(vittone)
Comment on attachment 8369910 [details] Subject but empty body mms I think the result is just fine. I'd would include it in 1.3 because thread's visual refresh for 1.4 is reduced to color changes. Good job guys!
Attachment #8369910 - Flags: review?(vittone) → review+
Comment on attachment 8361485 [details] Link to github NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings. [Approval Request Comment] [Bug caused by] (feature/regressing bug #): When sending a MMS with subject but without body the UI was not working as expected. [User impact] if declined: Wrong UI [Testing completed]: Working fine in my tests [Risk to taking this patch] (and alternatives if risky): This patch is not risky due to is surgical, and only modify one scenario for avoiding the wrong UI rendering when sending a MMS with Subject but without Body. Safe to land. [String changes made]: none
Attachment #8361485 - Flags: approval-gaia-v1.3?(fabrice)
Attachment #8361485 - Flags: approval-gaia-v1.3?(fabrice) → approval-gaia-v1.3+
Borja or Steve, Can either of you explain why the `noAttachment` case was lumped into the "message without subject" case? I've read through the history in this bug and I don't see where Ayman signed off on rolling these into one. Relevant comments: https://github.com/mozilla-b2g/gaia/commit/014e6c39c69fdc51c08079c9bb3b827664387dcd#diff-1
(In reply to Rick Waldron [:rwaldron] from comment #48) > Borja or Steve, > > Can either of you explain why the `noAttachment` case was lumped into the > "message without subject" case? I've read through the history in this bug > and I don't see where Ayman signed off on rolling these into one. > Hi Rick, Before v1.3, we didn't implement subject feature for mms(neither edit nor display it). If the mms has no attachments(media and text), we treat it as an error case and display error message inside the message bubble with error style. But mms without attachment is validate if the message have subject. Once subject feature landed in v1.3, we should also consider the subject for handling different cases as well. So the flow will become: Before v1.3: If mms has no attachments -> display error message inside the bubble with error style After v1.3: a) mms has no attachments and no subject -> display error message inside the bubble with error style b) mms has no attachments but subject exist -> display empty message. So we didn't lumped into "message without subject" case. It divide into 2 different cases based on the subject actually. This bug described that we treat (b) as an error case because of outdated logic for handling no attachment case. We need ux input here for (b) and José provided his viewpoint in comment 31.
Attachment #8361576 - Flags: feedback?(aymanmaat) → feedback+
Hi John, Fabrice gave the approval-gaia-v1.3+, could you please help us with the uplift to v1.3 branch?. Thanks!
Flags: needinfo?(jhford)
Uplifted 014e6c39c69fdc51c08079c9bb3b827664387dcd to: v1.3: 31a2c3539e8cf0982d310f521809e63c7303d9f8
Flags: needinfo?(jhford)
Tested 02/06/2014 and working 1.3 Gecko 1fe8aca Gaia fb35a67 Tested 02/06/2014 and working master Gecko fdde61f Gaia 1aa8377
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: