Closed Bug 876746 Opened 11 years ago Closed 11 years ago

[MMS] Reword the delivery report settings and handle multiple recipients delivery status properly

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(blocking-b2g:koi+)

RESOLVED FIXED
blocking-b2g koi+

People

(Reporter: gerard-majax, Assigned: steveck)

References

Details

(Whiteboard: [u=commsapps-user c=messaging p=1])

Attachments

(1 file, 1 obsolete file)

Sending a MMS from a B2G device to an Android device, when it is opened on the Android 2.3.3, the user is prompted with a delivery report confirmation. On the B2G device, "delivery reports" in Messages settings is OFF.

Reproduced with gecko b2g18 and gaia master.
Steps to reproduce:

0. Ensure MMS are enabled and "Delivery reports" are disabled.
1. Send a MMS from B2G.
2. Receive it on another device.

Expected result:
The receiver should not be sending any delivery report.

Actual result:
The receiver is prompted to send a delivery report.
Hi Alexandre,
Delivery reports is only for sms message currently. MMS delivery report is still implementing in bug 850140.
Depends on: 850140
And, the only thing we need to do after bug 850140 is rewording the settings title. The key for sms and mms delivery report should be the same.
Assignee: nobody → schung
So let's move this bug to the settings app then.
Component: Gaia::SMS → Gaia::Settings
Summary: [MMS] MMS sent from B2G triggers delivery report even when set off in settings → [MMS] Reword the delivery report settings once it is implemented for MMS
logcat after a successfull MMS sent:

I/Gecko   (   77): -@- MmsService: receiveWapPush: msg = {"headers":{"x-mms-message-type":134,"x-mms-mms-version":19,"message-id":"DeyAjshCEeKbkwAmufq69A","to":{"address":"+336xxxx","type":"PLMN"},"date":"2013-05-29T09:28:19.000Z","x-mms-status":129},"type":134}
I/Gecko   (   77): -@- MmsService: handleDeliveryIndication: got delivery report for DeyAjshCEeKbkwAmufq69A
(In reply to Alexandre LISSY :gerard-majax from comment #5)
> logcat after a successfull MMS sent:
> I/Gecko   (   77): -@- MmsService: handleDeliveryIndication: got delivery
> report for DeyAjshCEeKbkwAmufq69A

Yeap, we can receive that but not yet handle that. Whether to implement the delivery report for MMS is still a question mark to me.
bug 850140 is not blocking, so blocking- here.
blocking-b2g: leo? → -
Since bug 850140 is koi+ now, renominate this bug for mms delivery report necessary changes in gaia.
blocking-b2g: - → koi?
Blocks: 899025
Summary: [MMS] Reword the delivery report settings once it is implemented for MMS → [MMS] Reword the delivery report settings and handle multiple recipients delivery status properly
Hi Gene, I have a patch for all message delivery report, but I changed the settings key from ril.sms to ril.messaging for general case. If you think the changes is fine, it may need your help to integrate sms/mms to messaging only, thanks.
Attachment #785825 - Flags: feedback?(gene.lian)
Attachment #785825 - Flags: feedback?(gene.lian) → feedback+
Comment on attachment 785825 [details] [diff] [review]
0001-Bug-876746-Gaia-changes-for-MMS-Delivery-report-supp.patch

Review of attachment 785825 [details] [diff] [review]:
-----------------------------------------------------------------

Sorry, please see bug 850140 comment 40.  I think Gecko is only responsible for mechanisms, not policies.  We provide the choices, and Gaia is the one that has to make the decision.  "I can only show you the door. You're the one that has to walk through it." ~ Oracle, "The Matrix".
Attachment #785825 - Flags: feedback+ → feedback-
Actually, in my first thought I agree with Vicamo's point. I did that because Gaia was saying they can only bind one setting to one key at one time. However, it should be just an implementation issue on the Gaia end. Indeed, Gaia should try to find a way to handle that.
Attached file pull-request.html
Hi, Borja, I update the delivery report feature for mms with some test cases. Although Ayman might not have free cycle for the possible new wireframe, but I think the changes in this patch is needed. We can create a follow up issue for the new wireframe later.

Hi Kaze, I also made some changes for setting the key manually in dialog. Could you please take a look for this part?

Thanks.
Attachment #785825 - Attachment is obsolete: true
Attachment #787603 - Flags: review?(kaze)
Attachment #787603 - Flags: review?(fbsc)
Testing & reviewing! :)
Small comments and from my side R+. Let me know when ready and I'll take a look again before R+ officially in bugzilla!
Hi Borja, I've update the patch, please take a look again, thank!
Comment on attachment 787603 [details]
pull-request.html

Thanks for the patch! Delivery for MMS Ready! :)
Attachment #787603 - Flags: review?(fbsc) → review+
Dear mozilla colleagues, mark leo? because it's GCF certification case.
blocking-b2g: koi? → leo?
if this is due for leo, then it's late-l10n because there are test changes.

Unless we can have a leo-specific version without l10n changes. Steve, do you think it's possible ?
Flags: needinfo?(schung)
Keywords: late-l10n
I'm afraid that l10n changes is necessary because the wording in settings is specific to "text message". Having the ability to handle mms delivery report is not a big problem since patch is already here. If we really want this feature in leo without l10n change, the only way in my mind is removeing the title(Text message confirmation) and detail description text(Request notification of delivery...).
Flags: needinfo?(schung)
too late to change for v1.1. Can this be waived for v1.1?
move this to v1.2
blocking-b2g: leo? → koi+
Flags: needinfo?(liuyongming)
Whiteboard: [u=commsapps-user c=messaging p=0]
Comment on attachment 787603 [details]
pull-request.html

Hi Julien, thanks for he comments, I updated the patch. Since I could not find Kaze this week for the settings part review, I'll ask Arthur for settings review.
Attachment #787603 - Flags: review?(kaze) → review?(arthur.chen)
koi, so not late-l10n after all ;)
Keywords: late-l10n
Comment on attachment 787603 [details]
pull-request.html

Steve, thanks for the patch. It is not suggested to have panel specific logic placed in settings.js. Details please refer to github. Thanks!
Attachment #787603 - Flags: review?(arthur.chen)
It's ok for us to have this feature in v1.2.
Flags: needinfo?(liuyongming)
Whiteboard: [u=commsapps-user c=messaging p=0] → [u=commsapps-user c=messaging p=1]
Comment on attachment 787603 [details]
pull-request.html

Hi Arthur, I update the patch based on our discussion last week. Thanks for the suggestion.
Attachment #787603 - Flags: review?(arthur.chen)
Comment on attachment 787603 [details]
pull-request.html

r=me. Thanks for the patch!
Attachment #787603 - Flags: review?(arthur.chen) → review+
Merged in master : d7802cbe1a639f675fb6448949049b84e240c20c
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: