Closed Bug 920164 Opened 11 years ago Closed 11 years ago

[MMS] Display the subject of an MMS, if present

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(blocking-b2g:1.3+)

RESOLVED FIXED
1.3 Sprint 5 - 11/22
blocking-b2g 1.3+

People

(Reporter: julienw, Assigned: evhan55)

References

Details

(Whiteboard: MMS_TEF, [u=commsapps-user c=messaging p=0] )

Attachments

(4 files, 1 obsolete file)

We have to show the subject in the MMS App when receiving a MMS.
NI Ayman for the Wireframes.
Flags: needinfo?(aymanmaat)
Assignee: nobody → evelyn
(In reply to Julien Wajsberg [:julienw] (in Paris-Web conference thursday and friday) from comment #0) > We have to show the subject in the MMS App when receiving a MMS. Just to clarify, this bug is also supposed to handle displaying subjects for MMS's that are being sent, as well as received, right? I am not talking about composing subjects, but I am talking about displaying subjects on MMS's I have composed before. If so, I'd like to change the title of this bug.
(In reply to Evelyn Eastmond [:evhan55] from comment #2) > (In reply to Julien Wajsberg [:julienw] (in Paris-Web conference thursday > and friday) from comment #0) > > We have to show the subject in the MMS App when receiving a MMS. > > Just to clarify, this bug is also supposed to handle displaying subjects for > MMS's that are being sent, as well as received, right? Yes, per these "wireframes" https://bugzilla.mozilla.org/show_bug.cgi?id=919966#c5 Any message should display a subject if the subject property of a message object has a value that is not an empty string, null or undefined
(In reply to Rick Waldron from comment #3) > (In reply to Evelyn Eastmond [:evhan55] from comment #2) > > (In reply to Julien Wajsberg [:julienw] (in Paris-Web conference thursday > > and friday) from comment #0) > > > We have to show the subject in the MMS App when receiving a MMS. > > > > Just to clarify, this bug is also supposed to handle displaying subjects for > > MMS's that are being sent, as well as received, right? > > Yes, per these "wireframes" > https://bugzilla.mozilla.org/show_bug.cgi?id=919966#c5 > > Any message should display a subject if the subject property of a message > object has a value that is not an empty string, null or undefined Actually, for MMS's from my Samsung Galaxy S4 that I have been testing with, MMS's with no subject get a subject of 'no subject'. So I should test for that too? And who knows what other carriers do :( Ok, I will change the name of this bug to accurately reflect that it's for the rendering of both sent and received MMS's.
Summary: [MMS] Display the subject of a received MMS, if present → [MMS] Display the subject of an MMS, if present
(In reply to Evelyn Eastmond [:evhan55] from comment #4) > Actually, for MMS's from my Samsung Galaxy S4 that I have been testing with, > MMS's with no subject get a subject of 'no subject'. So I should test for > that too? HA! Yes, absolutely.
(In reply to Evelyn Eastmond [:evhan55] from comment #4) > (In reply to Rick Waldron from comment #3) > > (In reply to Evelyn Eastmond [:evhan55] from comment #2) > > > (In reply to Julien Wajsberg [:julienw] (in Paris-Web conference thursday > > > and friday) from comment #0) > > > > We have to show the subject in the MMS App when receiving a MMS. > > > > > > Just to clarify, this bug is also supposed to handle displaying subjects for > > > MMS's that are being sent, as well as received, right? > > > > Yes, per these "wireframes" > > https://bugzilla.mozilla.org/show_bug.cgi?id=919966#c5 > > > > Any message should display a subject if the subject property of a message > > object has a value that is not an empty string, null or undefined > > Actually, for MMS's from my Samsung Galaxy S4 that I have been testing with, > MMS's with no subject get a subject of 'no subject'. So I should test for > that too? And who knows what other carriers do :( > > Ok, I will change the name of this bug to accurately reflect that it's for > the rendering of both sent and received MMS's. Hi Evelyn, Rick When an MMS is received with no subject can you ensure that the text 'no subject' is not output into the message thread. I have been quit deliberate in my specification of this because real estate is at a premium on our devices therefore any unnecessary content on the screen should be avoided in order to maximise the space for the information contained in the message thread (the messages themselves and subjects where a subject is added). 'no message' is considered data rather than information. feel free to ni? me if you wish to discuss futher
Flags: needinfo?(aymanmaat)
(In reply to ayman maat :maat from comment #6) > (In reply to Evelyn Eastmond [:evhan55] from comment #4) > > (In reply to Rick Waldron from comment #3) > > > (In reply to Evelyn Eastmond [:evhan55] from comment #2) > > > > (In reply to Julien Wajsberg [:julienw] (in Paris-Web conference thursday > > > > and friday) from comment #0) > > > > > We have to show the subject in the MMS App when receiving a MMS. > > > > > > > > Just to clarify, this bug is also supposed to handle displaying subjects for > > > > MMS's that are being sent, as well as received, right? > > > > > > Yes, per these "wireframes" > > > https://bugzilla.mozilla.org/show_bug.cgi?id=919966#c5 > > > > > > Any message should display a subject if the subject property of a message > > > object has a value that is not an empty string, null or undefined > > > > Actually, for MMS's from my Samsung Galaxy S4 that I have been testing with, > > MMS's with no subject get a subject of 'no subject'. So I should test for > > that too? And who knows what other carriers do :( > > > > Ok, I will change the name of this bug to accurately reflect that it's for > > the rendering of both sent and received MMS's. > > Hi Evelyn, Rick > > When an MMS is received with no subject can you ensure that the text 'no > subject' is not output into the message thread. I have been quit deliberate > in my specification of this because real estate is at a premium on our > devices therefore any unnecessary content on the screen should be avoided in > order to maximise the space for the information contained in the message > thread (the messages themselves and subjects where a subject is added). 'no > message' is considered data rather than information. > > feel free to ni? me if you wish to discuss futher Hi Ayman, Yes, we will filter any meta-data/information out and only display subjects and messages that have content. I think we are in agreement. Sorry for the misunderstanding. Do we know what kinds of meta-data carriers are likely to send? 'no subject' was something I noticed on a test with a specific kind of device on a specific carrier. I am not sure if other devices/carriers would send other meta-data kind of subjects. Thanks, Evelyn
Hi Rick, My first go at this patch is almost there. It is missing two features: 1) Proper filtering of (localized) subjects that we don't wan to display 2) Concatenation of subjects that are too long to fit on one line (this might be an NI from the UX/UI peeps) For 1), I'd like to get your advice on how to do it better than my initial try: https://github.com/evhan55/gaia/compare/920164#diff-74841e428dea67f7357f1893a4d26155R1203 https://github.com/evhan55/gaia/compare/920164#diff-74841e428dea67f7357f1893a4d26155R1224 Any advice appreciated, thanks!
Attachment #815960 - Flags: feedback?(waldron.rick)
(In reply to Evelyn Eastmond [:evhan55] from comment #8) > Created attachment 815960 [details] > https://github.com/evhan55/gaia/compare/920164 > > Hi Rick, > > My first go at this patch is almost there. > It is missing two features: > > 1) Proper filtering of (localized) subjects that we don't wan to display > 2) Concatenation of subjects that are too long to fit on one line (this > might be an NI from the UX/UI peeps) > > For 1), I'd like to get your advice on how to do it better than my initial > try: > https://github.com/evhan55/gaia/compare/920164#diff- > 74841e428dea67f7357f1893a4d26155R1203 > https://github.com/evhan55/gaia/compare/920164#diff- > 74841e428dea67f7357f1893a4d26155R1224 > > Any advice appreciated, thanks! By 'concatenation' I meant 'truncation' or shortening with trailing ellipses, my bad.
> > Do we know what kinds of meta-data carriers are likely to send? 'no > subject' was something I noticed on a test with a specific kind of device on > a specific carrier. I am not sure if other devices/carriers would send > other meta-data kind of subjects. > I dont, but i can find out. ni? to me to track down this information
Flags: needinfo?(aymanmaat)
Hi Ayman, We need more information regarding MMS subject text that can possibly come from carriers/devices that we don't want to display, but aren't necessarily empty. For example, for a Samsung Galaxy S4 on Sprint in the US, sending an MMS with no subject, sends a subject line of 'no subject'. Thank you! Evelyn
> 2) Concatenation of subjects that are too long to fit on one line (this > might be an NI from the UX/UI peeps) I will look at your patch tomorrow (its friday night here in Berlin, and can hear the clatter of beer bottles calling my name) ...could I ask in what instance you are truncating subject? We should not do this in the message thread itself. We should display the full subject.
Flags: needinfo?(evelyn)
> ...could I ask in what instance you are truncating subject? We should not do > this in the message thread itself. We should display the full subject. Yep ok! Is there a mockup with a multi-line subject? Some of them can be too long to fit in one line. All the subjects in the mockups here: https://bugzilla.mozilla.org/show_bug.cgi?id=919966#c5 are single-line subjects.
Flags: needinfo?(evelyn)
Attached image 920164.png (deleted) —
(In reply to Evelyn Eastmond [:evhan55] from comment #8) > Created attachment 815960 [details] > https://github.com/evhan55/gaia/compare/920164 > > Hi Rick, > > My first go at this patch is almost there. > It is missing two features: > > 1) Proper filtering of (localized) subjects that we don't wan to display I don't know what the plan is. > 2) Concatenation of subjects that are too long to fit on one line (this > might be an NI from the UX/UI peeps) This should work: white-space: nowrap; overflow: hidden; text-overflow: ellipsis; @ayman, I've attached a screenshot of progress so far
(In reply to Evelyn Eastmond [:evhan55] from comment #13) > > ...could I ask in what instance you are truncating subject? We should not do > > this in the message thread itself. We should display the full subject. > > Yep ok! Is there a mockup with a multi-line subject? Some of them can be > too long to fit in one line. All the subjects in the mockups here: > https://bugzilla.mozilla.org/show_bug.cgi?id=919966#c5 are single-line > subjects. Evelyn, Ayman—what about "tap to show" and "tap to hide" the remainder of the subject?
> Evelyn, Ayman—what about "tap to show" and "tap to hide" the remainder of > the subject? This sounds ok to me, I'm wondering what the design would be to show users they can do that without taking up too much space. Ellipses to cut off the message and then a disclosure triangle? Only ellipses? A button? I'm ok with just showing the entire subject too, although that makes most sense only if subjects have a character limit.
(In reply to Evelyn Eastmond [:evhan55] from comment #16) > > Evelyn, Ayman—what about "tap to show" and "tap to hide" the remainder of > > the subject? > > This sounds ok to me, I'm wondering what the design would be to show users > they can do that without taking up too much space. Ellipses to cut off the > message and then a disclosure triangle? Only ellipses? A button? > > I'm ok with just showing the entire subject too, although that makes most > sense only if subjects have a character limit. Character limit of subject is 40 characters i believe. I would prefer to show the whole subject and not have a tap to show/hide feature.
(In reply to ayman maat :maat from comment #17) > Character limit of subject is 40 characters i believe. I would prefer to > show the whole subject and not have a tap to show/hide feature. Okay, and what if the 40-character subject is too wide to fit on one line?
Can we check if the 'no subject' string is something spec-ed ? I'm especially worried that Android send other strings when using other locales, eg french...
(In reply to Julien Wajsberg [:julienw] from comment #19) > Can we check if the 'no subject' string is something spec-ed ? I'm > especially worried that Android send other strings when using other locales, > eg french... I think Ayman has been NI'd on this issue, to research this and see what strings we will need to look out for.
(In reply to Evelyn Eastmond [:evhan55] from comment #20) > (In reply to Julien Wajsberg [:julienw] from comment #19) > > Can we check if the 'no subject' string is something spec-ed ? I'm > > especially worried that Android send other strings when using other locales, > > eg french... > > I think Ayman has been NI'd on this issue, to research this and see what > strings we will need to look out for. Yep, i am making enquiries, but in the meantime if anyone else has any example strings please feel free to post. leaving ni? for me
(In reply to Evelyn Eastmond [:evhan55] from comment #18) > (In reply to ayman maat :maat from comment #17) > > Character limit of subject is 40 characters i believe. I would prefer to > > show the whole subject and not have a tap to show/hide feature. > > Okay, and what if the 40-character subject is too wide to fit on one line? referencing: FFOS_MessageSubject_V1.3_20121009_V4.0 **for message thread** Page 6 frame 2 'Message thread created with subject in the message' annotation 01: "Whatever was written in the subject field in screen one is output here. If the text length is more than one line the subject text wraps into two lines. It is not truncated." **for subject field** Page 9 frame 'Content in subject field' "If the content that is added to the subject field is more than one line extendthe subject field onto two lines and push the message field downwards."
Flags: needinfo?(aymanmaat)
(In reply to ayman maat :maat from comment #22) > referencing: FFOS_MessageSubject_V1.3_20121009_V4.0 Thank you, Ayman. Is this a document I can access somewhere? I don't see it on the Mozilla UX box.com account.
It's in bug 885680. (I think it should be in at least the feature bug...)
(In reply to Julien Wajsberg [:julienw] from comment #24) > It's in bug 885680. (I think it should be in at least the feature bug...) Thank you, I should have looked there first for all these questions I have asked. I see it now.
(In reply to Evelyn Eastmond [:evhan55] from comment #25) > (In reply to Julien Wajsberg [:julienw] from comment #24) > > It's in bug 885680. (I think it should be in at least the feature bug...) > > Thank you, I should have looked there first for all these questions I have > asked. I see it now. your quite right though, its not on box. I thought i had uploaded it there! Sorry! anyway. i have uploaded it to box here: Firefox UX OS > 1.3 Specs > Messages app
Attached image 920164-long_subject.png (obsolete) (deleted) —
What should the line-height be for long subjects that go on multiple lines? See attached image. Might take a while to get this CSS right for both single-line and multi-line subjects
Flags: needinfo?(aymanmaat)
Attached image 920164_long-subject-2.png (deleted) —
Trying styles for long subject again, please review this screenshot instead.
Attachment #816700 - Attachment is obsolete: true
(In reply to Evelyn Eastmond [:evhan55] from comment #27) > Created attachment 816700 [details] > 920164-long_subject.png > > What should the line-height be for long subjects that go on multiple lines? > See attached image. Might take a while to get this CSS right for both > single-line and multi-line subjects This is more a question for the visual designer so i am going to ni? Victoria to advise you. Victoria, could you also give Evelyn feedback on Comment 28 as well as comment 27 please. Thanks.
Flags: needinfo?(aymanmaat) → needinfo?(vpg)
PLease refer to this guide for the line height: http://www.mozilla.org/en-US/styleguide/products/firefox-os/typeface/ Also, please do not forget the margin at the end of the bubble should be 1.5 rem so the text should not extend that far. Thanks
Flags: needinfo?(vpg)
PLease refer to this guide for the line height: http://www.mozilla.org/en-US/styleguide/products/firefox-os/typeface/ Also, please do not forget the margin at the end of the bubble should be 1.5 rem so the text should not extend that far. Thanks
(In reply to Victoria Gerchinhoren from comment #31) > PLease refer to this guide for the line height: > http://www.mozilla.org/en-US/styleguide/products/firefox-os/typeface/ Thank you. I don't see any specs there regarding line-height, unless I am missing something? I tried inspecting the paragraphs of Fira Sans text, but they are actually images and not pure HTML. > Also, please do not forget the margin at the end of the bubble should be 1.5 > rem so the text should not extend that far. Thanks, I didn't realize it needed to be on both sides. I will flag you when there is a new implementation to review.
> Also, please do not forget the margin at the end of the bubble should be 1.5 > rem so the text should not extend that far. Looking at this mockup https://bug919966.bugzilla.mozilla.org/attachment.cgi?id=814143 I don't see the 1.5rem margin on the right hand-side. Do you know what size the right-side margin (on incoming messages) should be?
Flags: needinfo?(vpg)
(In reply to Evelyn Eastmond [:evhan55] from comment #33) > > Also, please do not forget the margin at the end of the bubble should be 1.5 > > rem so the text should not extend that far. > > Looking at this mockup > https://bug919966.bugzilla.mozilla.org/attachment.cgi?id=814143 > > I don't see the 1.5rem margin on the right hand-side. Do you know what size > the right-side margin (on incoming messages) should be? Hi Victoria, I see now, it's 1.5rem on the right and 4.0rem on the left. I'll try to cancel the NI.
Flags: needinfo?(vpg)
Flags: needinfo?(aymanmaat)
Thanks for that Evelyn ;)
Evelyn, are you still expecting something from Ayman here ?
(In reply to Julien Wajsberg [:julienw] from comment #36) > Evelyn, are you still expecting something from Ayman here ? Yes, Ayman was going to research possible empty subject strings that we need to filter out and not display. For example, some carriers send along a 'no subject' message. It would be good to get more insight on that.
(In reply to Evelyn Eastmond [:evhan55] from comment #37) > (In reply to Julien Wajsberg [:julienw] from comment #36) > > Evelyn, are you still expecting something from Ayman here ? > > Yes, Ayman was going to research possible empty subject strings that we need > to filter out and not display. For example, some carriers send along a 'no > subject' message. It would be good to get more insight on that. Sorry I am late replying. I am currently lead to believe that the presence of 'no subject' is actually device dependent not carrier dependant, so there is no need to filter. ni? me if you require more input
Flags: needinfo?(aymanmaat)
(In reply to ayman maat :maat from comment #38) > Sorry I am late replying. I am currently lead to believe that the presence > of 'no subject' is actually device dependent not carrier dependant, so there > is no need to filter. Just to clarify, you are ok with 'no subject' being displayed as the subject?
Flags: needinfo?(aymanmaat)
(In reply to Evelyn Eastmond [:evhan55] from comment #39) > (In reply to ayman maat :maat from comment #38) > > > Sorry I am late replying. I am currently lead to believe that the presence > > of 'no subject' is actually device dependent not carrier dependant, so there > > is no need to filter. > > Just to clarify, you are ok with 'no subject' being displayed as the subject? Hi Evelyn no, please do not display anything when there is no subject accompanying a message. what i was meaning when i said 'there is no need to filter' is that there are no strings to filter. The appearance of the string 'no subject' is down to the device, not the carrier, I am lead to believe. Therefore there should be no variation of strings sent to indicate that the message has no subject.
Flags: needinfo?(aymanmaat)
That means that if (hypothetically) a third-party device was sending the "no subject" string as a subject, we would display it. Am I understanding rightly?
Flags: needinfo?(aymanmaat)
(In reply to Julien Wajsberg [:julienw] from comment #41) > That means that if (hypothetically) a third-party device was sending the "no > subject" string as a subject, we would display it. Am I understanding > rightly? I would say so as we as the receiving device would not be able to distinguish between "no subject" being a string that the sending 'system' has added and a string the sender has added.
Flags: needinfo?(aymanmaat)
Great, thanks, all's clear now ;)
blocking a committed 1.3 user story. 1.3+
blocking-b2g: 1.3? → 1.3+
Ok, thanks for the clarification!
Target Milestone: --- → 1.3 Sprint 4 - 11/8
Target Milestone: 1.3 Sprint 4 - 11/8 → 1.3 Sprint 5 - 11/22
Comment on attachment 815960 [details] https://github.com/evhan55/gaia/compare/920164 Submitting for feedback. Questions: 1) I have filtered out 'system subjects' on MMS's that have subjects applied by the system. Not sure if my list is exhaustive? 2) What kind of test would suffice for this patch?
Attachment #815960 - Flags: feedback?(felash)
Attachment #815960 - Flags: feedback?(aymanmaat)
Evelyn, I don't exactly get what are these "system subjects", could you please explain? Thanks! Unit tests are probably enough :)
(In reply to Julien Wajsberg [:julienw] from comment #47) > Evelyn, I don't exactly get what are these "system subjects", could you > please explain? Thanks! I had noticed them in the MMS's with warnings in the Tom OHare SMS desktop mock thread. Looking through the code-base now, I'm not sure where those subjects are coming from. They are just being mocked and not actually generated by the system like I mistakenly believed. I will remove them. When you test the code, you will see what I mean in the Tom OHare test thread. > Unit tests are probably enough :) Ok I'll remove the filtering and add unit tests and resubmit for review.
@julienw, for context, the "Tom O'Hare" thread has messages whose sender value is "123456". I think those subjects might be made up?
Yep, that's what I was gonna say: you can change the subjects in the desktop mocks to put more sensible ones if you want :)
Comment on attachment 815960 [details] https://github.com/evhan55/gaia/compare/920164 removing flag to me as i think Julien has the questions asked on this covered. flag me again if there is something more you need from me.
Attachment #815960 - Flags: feedback?(aymanmaat) → feedback-
Attachment #815960 - Flags: feedback-
Comment on attachment 815960 [details] https://github.com/evhan55/gaia/compare/920164 Does not look bad :) I wonder if we really need these `.article-list[data-type="list"]` prefixes in the CSS... Long-term, we should move towards more componentized CSS, eg a message-bubble.css that would hold the css for a message bubble, so better start coding a more OO css already now, and I'm sure you know more than me about this :)
Attachment #815960 - Flags: feedback?(felash) → feedback+
(In reply to Julien Wajsberg [:julienw] from comment #52) > Comment on attachment 815960 [details] > https://github.com/evhan55/gaia/compare/920164 > > Does not look bad :) > > I wonder if we really need these `.article-list[data-type="list"]` prefixes > in the CSS... > > Long-term, we should move towards more componentized CSS, eg a > message-bubble.css that would hold the css for a message bubble, so better > start coding a more OO css already now, and I'm sure you know more than me > about this :) It looks like a lot of the CSS in style_unstable changed recently so I'm not sure how to think about it app-wide or system-wide yet
- `thread_ui.js` - added subject CSS class if message type is mms - added subject to the message DOM template if mms and has subject - `index.html` - added subject div and template item - `sms.css` - styling for subject div - `thread_ui_test.js` - added unit test to check against template interpolation with subject field for mms
Attachment #833311 - Flags: review?(felash)
Comment on attachment 833311 [details] https://github.com/mozilla-b2g/gaia/pull/13765 Resubmitting for review based on Julien's feedback
Comment on attachment 833311 [details] https://github.com/mozilla-b2g/gaia/pull/13765 Resubmitting for review based on Julien's feedback. Julien, I could not test this on the device because of the introduction of a new bug, which I had to file: https://bugzilla.mozilla.org/show_bug.cgi?id=940576 MMS's won't display until that bug is fixed. Should I take that bug?
Blocks: 941030
Comment on attachment 833311 [details] https://github.com/mozilla-b2g/gaia/pull/13765 r=me with the latest comments implemented Please ask me a review again if you don't implement them all :) Of course please have a green travis before merging! Thanks :)
Attachment #833311 - Flags: review?(felash) → review+
(In reply to Julien Wajsberg [:julienw] from comment #57) > Comment on attachment 833311 [details] > https://github.com/mozilla-b2g/gaia/pull/13765 > > r=me with the latest comments implemented > Please ask me a review again if you don't implement them all :) > > Of course please have a green travis before merging! > Thanks :) Should Rick do the merge if/when I implement all your requested changes?
just add the keyword "checkin-needed" in the bug, and somebody will eventually merge. (this could be Rick or me of course, if we happen to be available)
Keywords: checkin-needed
Status: NEW → RESOLVED
Closed: 11 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: