Closed Bug 816416 Opened 8 years ago Closed 8 years ago

B2G SMS: Use system messages to notify sent sms events

Categories

(Core :: DOM: Device Interfaces, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

()

RESOLVED FIXED
B2G C3 (12dec-1jan)
blocking-basecamp +
Tracking Status
firefox18 --- fixed
firefox19 --- fixed
firefox20 --- fixed

People

(Reporter: vicamo, Assigned: daleharvey)

References

Details

Attachments

(1 file, 1 obsolete file)

See bug 816110, emit system messages for SMS onsent in order to remove CostControl background service. See also bug 782489.
BB? for blocking bug 816110, a bb+ bug.
blocking-basecamp: --- → ?
Vicamo: How is this bug different from bug 816110?
Flags: needinfo?(vyang)
This bug is for Gecko, and bug 816110 is for Gaia.
Flags: needinfo?(vyang)
(In reply to Jonas Sicking (:sicking) from comment #2)
> Vicamo: How is this bug different from bug 816110?

It is not different, this only does not appear on possible duplicated. You can close the other one or add this as blocking the other one. ;)
You guys are sending mixed messages :)

If both this one and bug 816110 covers the gecko side of things, is there another bug filed on the gaia changes that are needed?
Assignee: nobody → dale
This sends more information than the cost control application necessarily needs, but I thought it would be a good idea to keep reasonably symmetrical with the sms-received system messages, the only difference is the additional segmentsMaxSeq property
Attachment #687063 - Flags: review?(htsai)
Ok, I understand. So this fixes the problem in Gecko and now, the bug #816110 says that Cost Control should use it. Is this what you mean? If so, as Vicamo says, then they are two different bugs.
Marking as a soft blocker. We may need to revisit this decision based on the decision for bug 816110.
blocking-basecamp: ? → -
Priority: -- → P4
Comment on attachment 687063 [details] [diff] [review]
Send system message on sms being sent

Refer the review to Vicamo because he knows much more sms impl. than I :)
Attachment #687063 - Flags: review?(htsai) → review?(vyang)
Comment on attachment 687063 [details] [diff] [review]
Send system message on sms being sent

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

Thank you for the patch, but please don't make any differences to SystemMessage and SmsManager events.

::: dom/system/gonk/RadioInterfaceLayer.js
@@ +1370,5 @@
> +    gSystemMessenger.broadcastMessage("sms-sent",
> +                                      {id: id,
> +                                       delivery: DOM_SMS_DELIVERY_SENT,
> +                                       deliveryStatus: RIL.GECKO_SMS_DELIVERY_STATUS_PENDING,
> +                                       receiver: options.number,

sender: message.sender || null,

@@ +1375,5 @@
> +                                       body: options.fullBody,
> +                                       messageClass: messageClass,
> +                                       timestamp: timestamp,
> +                                       read: true,
> +                                       segmentMaxSeq: options.segmentMaxSeq});

You don't need this. There is already a getNumberOfMessagesForText() API. I know it's stupid to invoke it again, but I don't really want to have any differences between messages got from SmsEvent and SystemMessage. If you feel like it's something necessary, please fire a new bug so that we can include this attribute in every received/sent messages and benefits every application despite of the event target registered.

@@ +2306,5 @@
>      options.envelopeId = this.createSmsEnvelope({request: request,
>                                                   number: options.number,
>                                                   fullBody: options.fullBody,
> +                                                 requestStatusReport: options.requestStatusReport,
> +                                                 segmentMaxSeq: options.segmentMaxSeq});

Ditto
Attachment #687063 - Flags: review?(vyang)
(In reply to Lawrence Mandel [:lmandel] from comment #8)
> Marking as a soft blocker. We may need to revisit this decision based on the
> decision for bug 816110.

It has been marked as blocking. Renominating.
Okay, based on the decision here and in the dependent bugs, blocking on this.
blocking-basecamp: ? → +
(In reply to Andrew Overholt [:overholt] from comment #12)
> Okay, based on the decision here and in the dependent bugs, blocking on this.

What priority? P1 or P2 or P3?
Priority: P4 → --
Nice thanks Vicamo, I should have realised that function was exposed to web content, patch is getting simpler :)
Attachment #687063 - Attachment is obsolete: true
Attachment #688746 - Flags: review?(vyang)
Comment on attachment 688746 [details] [diff] [review]
Send system message on sms being sent

Thank you so much :)
Attachment #688746 - Attachment is patch: true
Attachment #688746 - Flags: review?(vyang) → review+
Mass Modify: All un-milestoned, unresolved blocking-basecamp+ bugs are being moved into the C3 milestone. Note that the target milestone does not mean that these bugs can't be resolved prior to 12/10, rather C2 bugs should be prioritized ahead of C3 bugs.
Target Milestone: --- → B2G C3 (12dec-1jan)
https://hg.mozilla.org/mozilla-central/rev/09a07d2fbd23
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.