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)
Tracking
(blocking-b2g:-, b2g-v1.2 wontfix, b2g-v1.3 fixed, b2g-v1.4 fixed)
People
(Reporter: lolimartinezcr, Assigned: steveck)
Details
Attachments
(7 files)
(deleted),
image/png
|
Details | |
(deleted),
image/png
|
Details | |
(deleted),
image/png
|
Details | |
(deleted),
text/x-github-pull-request
|
borjasalguero
:
review+
fabrice
:
approval-gaia-v1.3+
|
Details |
(deleted),
image/png
|
Details | |
(deleted),
image/png
|
maat
:
feedback+
|
Details |
(deleted),
image/png
|
vittone
:
review+
|
Details |
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)
Reporter | ||
Updated•11 years ago
|
blocking-b2g: --- → 1.3?
Reporter | ||
Comment 1•11 years ago
|
||
Comment 2•11 years ago
|
||
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)
Comment 3•11 years ago
|
||
Only operation done to subject is exchanging multiple spaces and breaklines into single space
[dom.subject.value.replace(/\n\s*/g, ' ')]
Comment 4•11 years ago
|
||
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.
Comment 5•11 years ago
|
||
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.
Comment 6•11 years ago
|
||
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
Updated•11 years ago
|
QA Contact: jzimbrick
Comment 7•11 years ago
|
||
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
Comment 8•11 years ago
|
||
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
Comment 9•11 years ago
|
||
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.
Comment 10•11 years ago
|
||
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
Comment 11•11 years ago
|
||
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)
Comment 12•11 years ago
|
||
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!
Comment 13•11 years ago
|
||
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.
Comment 14•11 years ago
|
||
(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
Comment 15•11 years ago
|
||
Comment 16•11 years ago
|
||
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?
Comment 17•11 years ago
|
||
triage: 1.3+. broken new feature implementation
Updated•11 years ago
|
blocking-b2g: 1.3? → 1.3+
Assignee | ||
Comment 18•11 years ago
|
||
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
Comment 19•11 years ago
|
||
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.
Updated•11 years ago
|
Target Milestone: --- → 1.3 C2/1.4 S2(17jan)
Assignee | ||
Comment 20•11 years ago
|
||
(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) → ---
Assignee | ||
Updated•11 years ago
|
Target Milestone: --- → 1.3 C2/1.4 S2(17jan)
Assignee | ||
Comment 21•11 years ago
|
||
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?
Assignee | ||
Comment 22•11 years ago
|
||
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)
Comment 23•11 years ago
|
||
FYI I just pinged Ayman by email so that he can have a quick look here.
Thanks to both of you :)
Comment 24•11 years ago
|
||
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!
Comment 25•11 years ago
|
||
Could this be bug 957084 ?
Assignee | ||
Comment 26•11 years ago
|
||
(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? :)
Assignee | ||
Comment 27•11 years ago
|
||
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)
Comment 28•11 years ago
|
||
referencing comment 27 passing ni? to Jose as this is more of a visual design question.
Flags: needinfo?(aymanmaat) → needinfo?(vittone)
Updated•11 years ago
|
Target Milestone: 1.3 C2/1.4 S2(17jan) → 1.3 C3/1.4 S3(31jan)
Comment 29•11 years ago
|
||
Passing ni to Ayman again since he has more info about the original spec.
Flags: needinfo?(vittone) → needinfo?(aymanmaat)
Assignee | ||
Comment 30•11 years ago
|
||
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)
Assignee | ||
Updated•11 years ago
|
Target Milestone: 1.3 C2/1.4 S2(17jan) → 1.3 C3/1.4 S3(31jan)
Comment 31•11 years ago
|
||
(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)
Comment 32•11 years ago
|
||
Sorry José, I don't understand why it is so "complicated" to show a message with a different style?
Comment 33•11 years ago
|
||
(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.
Comment 34•11 years ago
|
||
Oki, makes sense, thanks!
Assignee | ||
Comment 35•11 years ago
|
||
Hi Borja,
I update the patch based on José's comment in comment 33. Please review the patch for the final result, thanks!
Comment 36•11 years ago
|
||
We punted this from 1.2. Why is this necessary for 1.3? Renoming for more discussion.
blocking-b2g: 1.3+ → 1.3?
Comment 37•11 years ago
|
||
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.
Comment 38•11 years ago
|
||
(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.
Comment 39•11 years ago
|
||
Already in 1.2, would like to wait till 1.4 for a fix.
Unblocking for 1.3 now.
blocking-b2g: 1.3? → -
Updated•11 years ago
|
Attachment #8361485 -
Flags: review?(borja.bugzilla) → review+
Comment 40•11 years ago
|
||
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
Comment 41•11 years ago
|
||
> 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 ?
Comment 42•11 years ago
|
||
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.
Comment 43•11 years ago
|
||
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.
Comment 44•11 years ago
|
||
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 45•11 years ago
|
||
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 46•11 years ago
|
||
https://github.com/steveck-chung/gaia/commit/00c5ae4f29f48fab128ca85e888315ea361266d4
https://github.com/mozilla-b2g/gaia/commit/014e6c39c69fdc51c08079c9bb3b827664387dcd
R+. Merged in master.
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Comment 47•11 years ago
|
||
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)
Updated•11 years ago
|
Attachment #8361485 -
Flags: approval-gaia-v1.3?(fabrice) → approval-gaia-v1.3+
Comment 48•11 years ago
|
||
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
Assignee | ||
Comment 49•11 years ago
|
||
(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.
Assignee | ||
Updated•11 years ago
|
Updated•11 years ago
|
Attachment #8361576 -
Flags: feedback?(aymanmaat) → feedback+
Comment 50•11 years ago
|
||
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)
Comment 51•11 years ago
|
||
Uplifted 014e6c39c69fdc51c08079c9bb3b827664387dcd to:
v1.3: 31a2c3539e8cf0982d310f521809e63c7303d9f8
Flags: needinfo?(jhford)
Reporter | ||
Comment 52•11 years ago
|
||
Tested 02/06/2014 and working
1.3
Gecko 1fe8aca
Gaia fb35a67
Tested 02/06/2014 and working
master
Gecko fdde61f
Gaia 1aa8377
Reporter | ||
Updated•11 years ago
|
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•