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

Status: RESOLVED FIXED

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

Description

[Julien Wajsberg [:julienw]]

We have to show the subject in the MMS App when receiving a MMS.

Comment 1

[Julien Wajsberg [:julienw]]

NI Ayman for the Wireframes.

Comment 2

[Evelyn Eastmond [:evhan55]]

(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.

Comment 3

[Rick Waldron [:rwaldron]]

(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

Comment 4

[Evelyn Eastmond [:evhan55]]

(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.

Comment 5

[Rick Waldron [:rwaldron]]

(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.

Comment 6

[ayman maat :maat]

(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

Comment 7

[Evelyn Eastmond [:evhan55]]

(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

Comment 8

[Evelyn Eastmond [:evhan55]]

Attachment: 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!

Comment 9

[Evelyn Eastmond [:evhan55]]

(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.

Comment 10

[ayman maat :maat]

> 
> 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

Comment 11

[Evelyn Eastmond [:evhan55]]

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

Comment 12

[ayman maat :maat]

> 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.

Comment 13

[Evelyn Eastmond [:evhan55]]

> ...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.

Comment 14

[Rick Waldron [:rwaldron]]

Attachment: 920164.png

(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

Comment 15

[Rick Waldron [:rwaldron]]

(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?

Comment 16

[Evelyn Eastmond [:evhan55]]

> 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.

Comment 17

[ayman maat :maat]

(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.

Comment 18

[Evelyn Eastmond [:evhan55]]

(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?

Comment 19

[Julien Wajsberg [:julienw]]

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...

Comment 20

[Evelyn Eastmond [:evhan55]]

(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.

Comment 21

[ayman maat :maat]

(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

Comment 22

[ayman maat :maat]

(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."

Comment 23

[Evelyn Eastmond [:evhan55]]

(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.

Comment 24

[Julien Wajsberg [:julienw]]

It's in bug 885680. (I think it should be in at least the feature bug...)

Comment 25

[Evelyn Eastmond [:evhan55]]

(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.

Comment 26

[ayman maat :maat]

(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

Comment 27

[Evelyn Eastmond [:evhan55]]

Attachment: 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

Comment 28

[Evelyn Eastmond [:evhan55]]

Attachment: 920164_long-subject-2.png

Trying styles for long subject again, please review this screenshot instead.

Comment 29

[ayman maat :maat]

(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.

Comment 30

[Victoria Gerchinhoren [:vicky]]

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

Comment 31

[Victoria Gerchinhoren [:vicky]]

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

Comment 32

[Evelyn Eastmond [:evhan55]]

(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.

Comment 33

[Evelyn Eastmond [:evhan55]]

> 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?

Comment 34

[Evelyn Eastmond [:evhan55]]

(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.

Comment 35

[Victoria Gerchinhoren [:vicky]]

Thanks for that Evelyn ;)

Comment 36

[Julien Wajsberg [:julienw]]

Evelyn, are you still expecting something from Ayman here ?

Comment 37

[Evelyn Eastmond [:evhan55]]

(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.

Comment 38

[ayman maat :maat]

(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

Comment 39

[Evelyn Eastmond [:evhan55]]

(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?

Comment 40

[ayman maat :maat]

(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.

Comment 41

[Julien Wajsberg [:julienw]]

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?

Comment 42

[ayman maat :maat]

(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.

Comment 43

[Julien Wajsberg [:julienw]]

Great, thanks, all's clear now ;)

Comment 44

[Joe Cheng [:jcheng] (please needinfo)]

blocking a committed 1.3 user story. 1.3+

Comment 45

[Evelyn Eastmond [:evhan55]]

Ok, thanks for the clarification!

Comment 46

[Evelyn Eastmond [:evhan55]]

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?

Comment 47

[Julien Wajsberg [:julienw]]

Evelyn, I don't exactly get what are these "system subjects", could you please explain?  Thanks!

Unit tests are probably enough :)

Comment 48

[Evelyn Eastmond [:evhan55]]

(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.

Comment 49

[Rick Waldron [:rwaldron]]

@julienw, for context, the "Tom O'Hare" thread has messages whose sender value is "123456". I think those subjects might be made up?

Comment 50

[Julien Wajsberg [:julienw]]

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 51

[ayman maat :maat]

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.

Comment 52

[Julien Wajsberg [:julienw]]

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 :)

Comment 53

[Evelyn Eastmond [:evhan55]]

(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

Comment 54

[Evelyn Eastmond [:evhan55]]

Attachment: https://github.com/mozilla-b2g/gaia/pull/13765

- `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

Comment 55

[Evelyn Eastmond [:evhan55]]

Comment on attachment 833311 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/13765

Resubmitting for review based on Julien's feedback

Comment 56

[Evelyn Eastmond [:evhan55]]

Comment on attachment 833311 [details] [review]
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?

Comment 57

[Julien Wajsberg [:julienw]]

Comment on attachment 833311 [details] [review]
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 :)

Comment 58

[Evelyn Eastmond [:evhan55]]

(In reply to Julien Wajsberg [:julienw] from comment #57)
> Comment on attachment 833311 [details] [review]
> 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?

Comment 59

[Julien Wajsberg [:julienw]]

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)

Comment 60

[Ryan VanderMeulen [:RyanVM]]

Master: https://github.com/mozilla-b2g/gaia/commit/e7f4ac493dbf7f3c92553d78b6a5c11215b16bae