Baseed on message app WF spec(https://bug919977.bugzilla.mozilla.org/attachment.cgi?id=822328) p.18, we need to implement a new message report panel for showing the receiver and delivery report information.

See also bug 901453 and its dependencies. You might need to do some cleanup there.

Thanks for the reminder :)

Maybe you should dupe bug 901453 here ?

1.3+ for a committed 1.3 user story

WIP branch :https://github.com/steveck-chung/gaia/commit/df97fb2784ef5fb4446bf4ff03e08312ccc6aed0 patch :https://github.com/steveck-chung/gaia/commit/df97fb2784ef5fb4446bf4ff03e08312ccc6aed0.patch The functionality already works for most of cases, need visual support(I might create another bug for this) and further clean up. Some known issue need to be fixed later: - Hide attachment icon - sent timestamp (need ux input) - attachment total size - unit test and desktop mock test - ... Hi Julien, would you mind to take a quick look first? It would be great to discover any possible problem in this implementation.

Hi Julien, it's only a WIP since the visual assets and test case is not ready yet, but it would be great if you could provide some feedback before further implementation. The functionality is almost ready and it should be able to run on the device/desktop. The missing parts are: - Unit Test - New message status icon(Might create another bug for it) - Report update ability when status changed

Comment on attachment 831387 [details] Link to github I think it makes sense to base your work on top of bug 934531 work. I'll produce a first WIP patch today that you can cherry-pick (and give me feedback too! :) ).

Hey Steve, see https://github.com/mozilla-b2g/gaia/pull/13713 for picking the commit. Tests are not passing yet though :( Will work on this first priority tomorrow so that you have a safe ground.

Comment on attachment 831387 [details] Link to github Hi Julien, I've updated the patch based on your contact renderer and modulize infomration view for group/report view. The test case is still not ready yet but maybe you could provide some feedback to this new module and the changes in contact renderer, thanks!

After Bug 939302 lands, Gaia needs to have corresponding changes to reinterpret deliveryTimestamp as long-long number, instead of Date object.

Gene, can you add me and Steve on bug 939302?

Comment on attachment 831387 [details] Link to github I've added some comments on the PR, enough to keep you working.

I'd like to unblock bug 901457 which is not in the scope of this user story.

Comment on attachment 831387 [details] Link to github Hi Julien, thanks for the comments. I've update the patch with complete(I hope) test cases. There is still some remain tasks that I didn't apply in this patch: - proactive report notification - read report status - sent timestamp for received message report - removing the tick icon in the message bubble I'll create bugs for these issue separatly since they might not be the first prority right now, but I'll target thme for 1.3, thanks.

Ayman, José, we have some questions for you. Base is the Visual Design at https://bug919977.bugzilla.mozilla.org/attachment.cgi?id=8336250. * the "Recipients" title should probably be different for incoming messages. Should we switch to "Sender" in that case? (Remember we don't do Group MMS for incoming MMS) * we need actual colors and font sizes for the various elements * we need a visual design for recipients/sender that don't match any contact * we need the actual texts for the possible values for status and for delivery reports (read reports will be for a future patch). See technical values at [1] for SMS and [2] for MMS. Note: the "delivery" field is actually the "Status" field in the panel, the name is misleading. * Please explicit the Visual Design when there is no MMS subject. * Please explicit the Visual Design for SMS * Please explicit the Visual Design for the cases we have one recipient/sender (SMS, and MMS in non-group MMS mode) [1] http://mxr.mozilla.org/mozilla-central/source/dom/mobilemessage/interfaces/nsIDOMMozSmsMessage.idl#25 [2] http://mxr.mozilla.org/mozilla-central/source/dom/mobilemessage/interfaces/nsIDOMMozMmsMessage.idl#48 Thanks a lot, don't hesitate to ask if you have any question!

Ayman, José, please see the previous comment 16.

Ayman, José, another question for you: * if one of the field changed while the user reads the panel, should the panel be updated live? (this could be done in another bug later, but it would be good to know)

Comment on attachment 831387 [details] Link to github The patch looks very good already. Some comments on the pull request, nothing big.

move to 1.4

Hey Steve, I've just seen we have a more complete visual design here: https://bug919977.bugzilla.mozilla.org/attachment.cgi?id=8336257 But some questions are still left unanswered :)

(In reply to Julien Wajsberg [:julienw] from comment #16) > Ayman, José, we have some questions for you. Base is the Visual Design at > https://bug919977.bugzilla.mozilla.org/attachment.cgi?id=8336250. > > * the "Recipients" title should probably be different for incoming messages. > Should we switch to "Sender" in that case? (Remember we don't do Group MMS > for incoming MMS) reference wireframe pack: FFOS_MessageApp_V1.3_20131125_V8.0 page: 25 frame: 3. Incoming message report ‘Recipients’ title that is shown in the outgoing message report is specified here as ‘From’ for the incoming message report. > * we need the actual texts for the possible values for status and for > delivery reports (read reports will be for a future patch). See technical > values at [1] for SMS and [2] for MMS. Note: the "delivery" field is > actually the "Status" field in the panel, the name is misleading. reference wireframe pack: FFOS_MessageApp_V1.3_20131125_V8.0 page: 27 Annotation 01, 02, 03 Julien, let me know if this does not answer your questions.

(In reply to Julien Wajsberg [:julienw] from comment #18) > Ayman, José, another question for you: > * if one of the field changed while the user reads the panel, should the > panel be updated live? (this could be done in another bug later, but it > would be good to know) It would be lovely if we could update the report live as the user is looking at it. Julien, is this possible? If so I would like Jose to propose a little visual realisation that is specific to the content that is being updated to draw the users attention to it rather than just having the text change. ni? to Julien to confirm live update of report is technically possible / feasable

This will be possible in bug 933133 (we still need a Gecko support for now).

(In reply to Julien Wajsberg [:julienw] from comment #16) > * we need actual colors and font sizes for the various elements Yes, you can find them here: https://bug919977.bugzilla.mozilla.org/attachment.cgi?id=8344654 Also, you can download necessary iconography here: https://bugzilla.mozilla.org/attachment.cgi?id=8336252 > * we need a visual design for recipients/sender that don't match any contact https://bug919977.bugzilla.mozilla.org/attachment.cgi?id=8344651 > * Please explicit the Visual Design when there is no MMS subject. https://bug919977.bugzilla.mozilla.org/attachment.cgi?id=8344647 > * Please explicit the Visual Design for SMS https://bug919977.bugzilla.mozilla.org/attachment.cgi?id=8344652 > * Please explicit the Visual Design for the cases we have one > recipient/sender (SMS, and MMS in non-group MMS mode) It is just the same as multiple recipients/senders, but with just one contact on it. Please, let me know if something is missing.

(In reply to ayman maat :maat from comment #22) > (In reply to Julien Wajsberg [:julienw] from comment #16) > > Ayman, José, we have some questions for you. Base is the Visual Design at > > https://bug919977.bugzilla.mozilla.org/attachment.cgi?id=8336250. > > > > * the "Recipients" title should probably be different for incoming messages. > > Should we switch to "Sender" in that case? (Remember we don't do Group MMS > > for incoming MMS) > > reference wireframe pack: FFOS_MessageApp_V1.3_20131125_V8.0 > page: 25 > frame: 3. Incoming message report > ‘Recipients’ title that is shown in the outgoing message report is specified > here as ‘From’ for the incoming message report. In the WF "From" is not capitalized but I suppose it should be capitalized, right? > > > * we need the actual texts for the possible values for status and for > > delivery reports (read reports will be for a future patch). See technical > > values at [1] for SMS and [2] for MMS. Note: the "delivery" field is > > actually the "Status" field in the panel, the name is misleading. > > reference wireframe pack: FFOS_MessageApp_V1.3_20131125_V8.0 > page: 27 > Annotation 01, 02, 03 > > Julien, let me know if this does not answer your questions. I have several remarks for this panel: * "3) Unsuccessful: <output reason why message failed>" <= we can't do that now because Gecko does not store the reason for a failure. So we'd have to store it somewhere. * we miss the "not-downloaded" state for MMS * we miss the "error" state for a delivery report (I think that this can happen independantly to the SMS/MMS status).

(In reply to Julien Wajsberg [:julienw] from comment #26) > (In reply to ayman maat :maat from comment #22) > > (In reply to Julien Wajsberg [:julienw] from comment #16) > > > Ayman, José, we have some questions for you. Base is the Visual Design at > > > https://bug919977.bugzilla.mozilla.org/attachment.cgi?id=8336250. > > > > > > * the "Recipients" title should probably be different for incoming messages. > > > Should we switch to "Sender" in that case? (Remember we don't do Group MMS > > > for incoming MMS) > > > > reference wireframe pack: FFOS_MessageApp_V1.3_20131125_V8.0 > > page: 25 > > frame: 3. Incoming message report > > ‘Recipients’ title that is shown in the outgoing message report is specified > > here as ‘From’ for the incoming message report. > > In the WF "From" is not capitalized but I suppose it should be capitalized, > right? I believe the design guidelines are that we capitalise (wireframes do not necessarily follow visual design guidelines for text ;) ) > > > > > * we need the actual texts for the possible values for status and for > > > delivery reports (read reports will be for a future patch). See technical > > > values at [1] for SMS and [2] for MMS. Note: the "delivery" field is > > > actually the "Status" field in the panel, the name is misleading. > > > > reference wireframe pack: FFOS_MessageApp_V1.3_20131125_V8.0 > > page: 27 > > Annotation 01, 02, 03 > > > > Julien, let me know if this does not answer your questions. > > I have several remarks for this panel: > > * "3) Unsuccessful: <output reason why message failed>" <= we can't do that > now because Gecko does not store the reason for a failure. So we'd have to > store it somewhere. Yep I know. I asked steve chung to open a bug for this so that we could work towards it in future releases. We should not block this release for it though. > * we miss the "not-downloaded" state for MMS I am not certain that we would have this state. as far as I am aware an undownloaded MMS is a SMS notification. Let me check this and get back to. ni? to me > * we miss the "error" state for a delivery report (I think that this can > happen independantly to the SMS/MMS status). Could you confirm what the error states are for delivery reports? I was not aware that there were any the only states I was aware of were requested/received. ni? to julien

(In reply to ayman maat :maat from comment #27) > > > > In the WF "From" is not capitalized but I suppose it should be capitalized, > > right? > > I believe the design guidelines are that we capitalise (wireframes do not > necessarily follow visual design guidelines for text ;) ) The VD doesn't have the panel with "from" ;) > > * "3) Unsuccessful: <output reason why message failed>" <= we can't do that > > now because Gecko does not store the reason for a failure. So we'd have to > > store it somewhere. > > Yep I know. I asked steve chung to open a bug for this so that we could work > towards it in future releases. We should not block this release for it > though. Steve, is there a bug for this? > > * we miss the "not-downloaded" state for MMS > > I am not certain that we would have this state. as far as I am aware an > undownloaded MMS is a SMS notification. Let me check this and get back to. > > ni? to me Undownloaded MMS looks like a SMS for the user, but it's really not from the development PoV. So if you want it to look like a SMS we need detailed guidelines for this. > > > * we miss the "error" state for a delivery report (I think that this can > > happen independantly to the SMS/MMS status). > > Could you confirm what the error states are for delivery reports? I was not > aware that there were any the only states I was aware of were > requested/received. > > ni? to julien I don't think we have any additional error states for delivery reports. Let's ask Gene. Gene, besides the "delivery == 'error'" case, what are the possible reasons to have the "error" state for the deliveryStatus property?

(In reply to Julien Wajsberg [:julienw] from comment #28) > (In reply to ayman maat :maat from comment #27) > > > * "3) Unsuccessful: <output reason why message failed>" <= we can't do that > > > now because Gecko does not store the reason for a failure. So we'd have to > > > store it somewhere. > > > > Yep I know. I asked steve chung to open a bug for this so that we could work > > towards it in future releases. We should not block this release for it > > though. > > Steve, is there a bug for this? > Just checked with Gene. This will need some changes in webIDL, and the most of message/IM apps does not have the ability either. So stroing the error code per message is not in the future plan... > > > > > > > * we miss the "error" state for a delivery report (I think that this can > > > happen independantly to the SMS/MMS status). > > > > Could you confirm what the error states are for delivery reports? I was not > > aware that there were any the only states I was aware of were > > requested/received. > > > > ni? to julien > > I don't think we have any additional error states for delivery reports. > Let's ask Gene. Gene, besides the "delivery == 'error'" case, what are the > possible reasons to have the "error" state for the deliveryStatus property? delivery error and deliveryStatus error is different(defined in http://mxr.mozilla.org/mozilla-central/source/dom/mobilemessage/src/gonk/MmsService.js#87 and http://mxr.mozilla.org/mozilla-central/source/dom/mobilemessage/src/gonk/MmsService.js#91). A message could be delivered successfully(to server) but return error deliveryStatus if any problem between server and receiver. Gene might know some specific cases for error deliveryStatus.

(In reply to Steve Chung [:steveck] from comment #29) > (In reply to Julien Wajsberg [:julienw] from comment #28) > > (In reply to ayman maat :maat from comment #27) > > > > * "3) Unsuccessful: <output reason why message failed>" <= we can't do that > > > > now because Gecko does not store the reason for a failure. So we'd have to > > > > store it somewhere. > > > > > > Yep I know. I asked steve chung to open a bug for this so that we could work > > > towards it in future releases. We should not block this release for it > > > though. > > > > Steve, is there a bug for this? > > > Just checked with Gene. This will need some changes in webIDL, and the most > of message/IM apps does not have the ability either. So stroing the error > code per message is not in the future plan... Ok, but i still want a bug opening for this as, from a UX PoV, continuity and clarity of information delivery is a must and the current implementation is not in line with specification. Its not a must have, but its something we need to add to the backlog to be address whenever there is the opportunity... otherwise it is just lost in time

Comment on attachment 831387 [details] Link to github Hi Julien, I updated the patch again based on your previous coments. One thing that I don't understand is about https://github.com/mozilla-b2g/gaia/pull/13654#discussion_r8140329. If date format dateTimeFormat_%c in English is wrong, should be just fix the shared local property(https://github.com/mozilla-b2g/gaia/blob/master/shared/locales/date/date.en-US.properties#L197)? Or you think it's because the date format does not match visual requirement? I can also add this to the visual follow-up bug.

(In reply to Steve Chung [:steveck] from comment #31) > Comment on attachment 831387 [details] > Link to github > > Hi Julien, I updated the patch again based on your previous coments. One > thing that I don't understand is about > https://github.com/mozilla-b2g/gaia/pull/13654#discussion_r8140329. If date > format dateTimeFormat_%c in English is wrong, should be just fix the shared > local > property(https://github.com/mozilla-b2g/gaia/blob/master/shared/locales/date/ > date.en-US.properties#L197)? dateTimeFormat_%c does what it should: it's similar to the "%c" implementation in C. So there is nothing to fix here :) > Or you think it's because the date format does > not match visual requirement? Yep that's it! > I can also add this to the visual follow-up > bug. If you want, I don't mind!

(In reply to Steve Chung [:steveck] from comment #29) > > delivery error and deliveryStatus error is different(defined in > http://mxr.mozilla.org/mozilla-central/source/dom/mobilemessage/src/gonk/ > MmsService.js#87 and > http://mxr.mozilla.org/mozilla-central/source/dom/mobilemessage/src/gonk/ > MmsService.js#91). A message could be delivered successfully(to server) but > return error deliveryStatus if any problem between server and receiver. Gene > might know some specific cases for error deliveryStatus. That's xactly what I meant: deliveryStatus error happens when delivery is error, but also in other situations. And I'd like Gene to help us with explaining the other situations :)

(In reply to Julien Wajsberg [:julienw] from comment #28) > (In reply to ayman maat :maat from comment #27) > > > > * we miss the "not-downloaded" state for MMS > > > > I am not certain that we would have this state. as far as I am aware an > > undownloaded MMS is a SMS notification. Let me check this and get back to. > > > > ni? to me > > Undownloaded MMS looks like a SMS for the user, but it's really not from the > development PoV. So if you want it to look like a SMS we need detailed > guidelines for this. ok no problem, I was under the impression that the notification message to download content was actually an SMS. but i was wrong :) so you are right we should output the state of the received MMS with attachment in the report that reflects its state in the thread. So i would suggest we extend the state beyond ‘not downloaded’ to include the following states: 1) Not downloaded 2) Downloading 3) Download failed 4) Downloaded what do you think Julien? ni? to Jose to provide the visual assets

(In reply to Julien Wajsberg [:julienw] from comment #33) > (In reply to Steve Chung [:steveck] from comment #29) > > > > delivery error and deliveryStatus error is different(defined in > > http://mxr.mozilla.org/mozilla-central/source/dom/mobilemessage/src/gonk/ > > MmsService.js#87 and > > http://mxr.mozilla.org/mozilla-central/source/dom/mobilemessage/src/gonk/ > > MmsService.js#91). A message could be delivered successfully(to server) but > > return error deliveryStatus if any problem between server and receiver. Gene > > might know some specific cases for error deliveryStatus. > > That's xactly what I meant: deliveryStatus error happens when delivery is > error, but also in other situations. And I'd like Gene to help us with > explaining the other situations :) cool, well i have learnt something else :) ok, well i am presuming that when a report fails there is no way for a user to recover from that failure (as in ‘retry’). So in the case that we have a failure in the delivery report (and read report when we get there) we can just output: Delivery: report failed Read: report failed I have had a chat with José and he concluded that there is no need for visual design in these instances, as in we will not include iconography or alter font treatment.

The only icon missing is "Not downloaded" in the message report , I suggest reuse the icon from email named "actionicon_email_downloadattachment" (available in box) at a 50% of opacity. Reuse in order to not create something very specific here and lighten it up to reinfornce that is not tappable.

(In reply to Julien Wajsberg [:julienw] from comment #33) > That's xactly what I meant: deliveryStatus error happens when delivery is > error, but also in other situations. And I'd like Gene to help us with > explaining the other situations :) Taking MMS for example, we have plenty of types to specify the |message.deliveryStatus|: case MMS.MMS_PDU_STATUS_RETRIEVED: deliveryStatus = DELIVERY_STATUS_SUCCESS; break; case MMS.MMS_PDU_STATUS_EXPIRED: case MMS.MMS_PDU_STATUS_REJECTED: case MMS.MMS_PDU_STATUS_UNRECOGNISED: case MMS.MMS_PDU_STATUS_UNREACHABLE: deliveryStatus = DELIVERY_STATUS_REJECTED; break; case MMS.MMS_PDU_STATUS_DEFERRED: deliveryStatus = DELIVERY_STATUS_PENDING; break; case MMS.MMS_PDU_STATUS_INDETERMINATE: deliveryStatus = DELIVERY_STATUS_NOT_APPLICABLE; break; but we only fire events to Gaia for only two cases: 1. ondeliverysuccess event for MMS.MMS_PDU_STATUS_RETRIEVED 2. ondeliveryerror event for MMS.MMS_PDU_STATUS_REJECTED which means |ondeliverysuccess| is only triggered when the MMS is delivered for sure; |ondeliveryerror| is only triggered when the MMS is rejected to deliver for sure. We didn't fire |ondeliveryerror| for some failed cases. For example, when the MMS is expired on the server (receivers haven't downloaded it for a long time). It depends on how detailed our UI desires and how we interpret the |ondeliveryerror|.

Comment on attachment 831387 [details] Link to github I add more fixing for timestamp and add not-downloaded status in the report. Another visual polish bug created in Bug 949379.

(In reply to Gene Lian [:gene] (needinfo? encouraged) from comment #37) > (In reply to Julien Wajsberg [:julienw] from comment #33) > > That's xactly what I meant: deliveryStatus error happens when delivery is > > error, but also in other situations. And I'd like Gene to help us with > > explaining the other situations :) > > Taking MMS for example, we have plenty of types to specify the > |message.deliveryStatus|: > > case MMS.MMS_PDU_STATUS_RETRIEVED: > deliveryStatus = DELIVERY_STATUS_SUCCESS; > break; > case MMS.MMS_PDU_STATUS_EXPIRED: > case MMS.MMS_PDU_STATUS_REJECTED: > case MMS.MMS_PDU_STATUS_UNRECOGNISED: > case MMS.MMS_PDU_STATUS_UNREACHABLE: > deliveryStatus = DELIVERY_STATUS_REJECTED; > break; > case MMS.MMS_PDU_STATUS_DEFERRED: > deliveryStatus = DELIVERY_STATUS_PENDING; > break; > case MMS.MMS_PDU_STATUS_INDETERMINATE: > deliveryStatus = DELIVERY_STATUS_NOT_APPLICABLE; > break; > > but we only fire events to Gaia for only two cases: > > 1. ondeliverysuccess event for MMS.MMS_PDU_STATUS_RETRIEVED > 2. ondeliveryerror event for MMS.MMS_PDU_STATUS_REJECTED > > which means |ondeliverysuccess| is only triggered when the MMS is delivered > for sure; |ondeliveryerror| is only triggered when the MMS is rejected to > deliver for sure. > > We didn't fire |ondeliveryerror| for some failed cases. For example, when > the MMS is expired on the server (receivers haven't downloaded it for a long > time). It depends on how detailed our UI desires and how we interpret the > |ondeliveryerror|. Gene, there is something I don't understand. I'm taking the SMS Message IDL [1] as reference for possible values (the MMS message IDL [2] has not a lot of documentation). So the messages can have a "deliveryStatus" with a value "error". Does this happen in the case "deliveryStatus = DELIVERY_STATUS_REJECTED" that you're saying above? Then I think "ondeliveryerror" should be fired as soon as we get a "error" value in the "deliveryStatus" field. For MMS it's not so easy because we have several deliveryInfo items... so I don't know here :/ In this bug, we're not doing anything on events anyway, we're only displaying what's in the message fields. Reacting to events will be bug 933133. [1] https://mxr.mozilla.org/mozilla-central/source/dom/mobilemessage/interfaces/nsIDOMMozSmsMessage.idl#31 [2] http://mxr.mozilla.org/mozilla-central/source/dom/mobilemessage/interfaces/nsIDOMMozMmsMessage.idl#18

Comment on attachment 831387 [details] Link to github I think we need one more round. some comments, but the biggest ones are: * the fact that we add the "hide" class to a lot of elements when we show the view, I think we can do better * we use a lot of ids in the markup, I'm not comfortable with this * the patch regresses the suggestion list display in the new message view

(In reply to Julien Wajsberg [:julienw] from comment #39) > I'm taking the SMS Message IDL [1] as reference for possible values (the MMS > message IDL [2] has not a lot of documentation). Yeap... I need to make up the IDL descriptions at bug 876267. > > So the messages can have a "deliveryStatus" with a value "error". Does this > happen in the case "deliveryStatus = DELIVERY_STATUS_REJECTED" that you're > saying above? No, not all of them. In the current implementation, we only fire ondeliveryerror event for: case MMS.MMS_PDU_STATUS_REJECTED: but not for: case MMS.MMS_PDU_STATUS_EXPIRED: case MMS.MMS_PDU_STATUS_UNRECOGNISED: case MMS.MMS_PDU_STATUS_UNREACHABLE: > > Then I think "ondeliveryerror" should be fired as soon as we get a "error" > value in the "deliveryStatus" field. Yes, I agree with you. We shouldn't limit it to MMS_PDU_STATUS_REJECTED only. In the W3C proposal, we don't do that either [1]. Will fire a bug for this in the next week. Anyway, it's not a critical issue, though. We can fix it for V1.4. [1] http://messaging.sysapps.org/#widl-MmsManager-ondeliveryerror > For MMS it's not so easy because we have several deliveryInfo items... so I > don't know here :/ Yeap... the ondeliveryerror will carry a full set of deliveryInfo items for all the receivers. Gaia needs to iterate through them to see which receiver reports the failed report.

Comment on attachment 831387 [details] Link to github Add more fix in the patch: * Apply 'information' class to 'thread-messages' instead of hiding specific elements * Replace most of ids with classes in information views * Revert the position of contact list to absolute and only apply static in report view. * Export ReportView and GrouView to global * Refine element naming with 'information' prefix * And more fixing based on previous comments... Contact list title and timestamp localize event handler(maybe we can refactor this part like moving to Utils in another bug ?) will be added in follow-up bug, thnaks.

Comment on attachment 831387 [details] Link to github The patch looks great! But I can't give r+ yet because there is a unit test failure ;) I think you need mocks for GroupView and ReportView and use them in the various test cases. Also there is jshint issues as well. And one useless nit in the unit test.

(In reply to Julien Wajsberg [:julienw] from comment #43) > Comment on attachment 831387 [details] > Link to github > > The patch looks great! > > But I can't give r+ yet because there is a unit test failure ;) I think you > need mocks for GroupView and ReportView and use them in the various test > cases. > > Also there is jshint issues as well. And one useless nit in the unit test. Sorry for the careless changes :s.. I fixed the test and jshint again, and Travis seems only failed in unrelated marionette test, thanks!

Comment on attachment 831387 [details] Link to github r=me yeah!

Thanks for all the detailed reviews! Landed in master: 503d54bbffa831c45654a251d19711de55c5f0a1 Hi Rick, since I've added some fixing based on your previous suggestion, if you still have other concern for this patch, I created a follow-up bug 949379 and any suggestion is welcome. Thnaks for the feedback you gave in this patch.