All users were logged out of Bugzilla on October 13th, 2018

B2G MMS: fail to read nsIDOMMozMmsMessage.receivers for a received MMS (a follow-up of bug 849741)

RESOLVED FIXED in Firefox 22

Status

()

RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: airpingu, Assigned: airpingu)

Tracking

Trunk
mozilla22
ARM
Gonk (Firefox OS)
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox20 wontfix, firefox21 wontfix, firefox22 fixed, b2g18+ fixed, b2g18-v1.0.0 wontfix, b2g18-v1.0.1 wontfix)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Assignee)

Description

6 years ago
This is a follow-up fix for bug 852911. Without this, the Gaia end would crash when trying to read |.receivers| for a received MMS. I believe this one should be a critical issue so leo+. Please let me know if anyone doesn't agree on this or ping me for more details. Thanks!
(Assignee)

Comment 1

6 years ago
(In reply to Gene Lian [:gene] from comment #0)
> This is a follow-up fix for bug 852911.
                              ^^^^^^^^^^ Sorry, it should be bug 849741.
Blocks: 849741
No longer blocks: 852911
(Assignee)

Updated

6 years ago
Summary: B2G MMS: fail to read nsIDOMMozMmsMessage.receivers for a received MMS (a follow-up of bug 852911) → B2G MMS: fail to read nsIDOMMozMmsMessage.receivers for a received MMS (a follow-up of bug 849741)
(Assignee)

Comment 2

6 years ago
Created attachment 728041 [details] [diff] [review]
Patch

This patch prevents us from falling into the following check at [1]:

NS_IMETHODIMP
MmsMessage::GetReceivers(JSContext* aCx, JS::Value* aReceivers)
{
  uint32_t length = mReceivers.Length();
  if (length == 0) {
    return NS_ERROR_UNEXPECTED;
  }
  // skip...
}

[1] http://mxr.mozilla.org/mozilla-central/source/dom/mobilemessage/src/MmsMessage.cpp#307
Attachment #728041 - Flags: review?(vyang)
Attachment #728041 - Flags: feedback?(ctai)
(Assignee)

Comment 3

6 years ago
Created attachment 728043 [details] [diff] [review]
Patch, V1.1

Fix a nit. Please see comment #2. Thanks!
Attachment #728041 - Attachment is obsolete: true
Attachment #728041 - Flags: review?(vyang)
Attachment #728041 - Flags: feedback?(ctai)
Attachment #728043 - Flags: review?(vyang)
Attachment #728043 - Flags: feedback?(ctai)
(Assignee)

Updated

6 years ago
Blocks: 850680
Comment on attachment 728043 [details] [diff] [review]
Patch, V1.1

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

There are still problems in handling incomplete receivers in notification indication PDUs. Let's open another follow-up bug and don't block Gaia developers for now.
Attachment #728043 - Flags: review?(vyang) → review+
(Assignee)

Comment 6

6 years ago
Nominate for leo+ because this one is a follow-up for bug 849741.
blocking-b2g: leo+ → leo?

Comment 7

6 years ago
https://hg.mozilla.org/mozilla-central/rev/52ae75e1909c
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
(Assignee)

Updated

6 years ago
blocking-b2g: leo? → ---
status-b2g18: --- → affected
tracking-b2g18: --- → ?
(Assignee)

Comment 8

6 years ago
We'll use b2g18-approval to uplift this after nominating for it.
(Assignee)

Comment 9

6 years ago
Comment on attachment 728043 [details] [diff] [review]
Patch, V1.1

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 849741
User impact if declined: without this, the Gaia end would crash to read the MMS' receivers info because we don't allow an MMS to contain no receivers. To solve this, we have to add an alternative receiver name at least even if we cannot get a valid phone number from ICC.
Testing completed: yes
Risk to taking this patch (and alternatives if risky): please don't hesitate to land this because it's just a follow-up for bug 849741 and only touches the MMS internal codes. I believe we don't have any risk to land this.
String or UUID changes made by this patch: no
Attachment #728043 - Flags: feedback?(ctai) → approval-mozilla-b2g18?
tracking-b2g18: ? → +
Comment on attachment 728043 [details] [diff] [review]
Patch, V1.1

Low risk forward fix that is self-contained in module - go ahead with uplift.
Attachment #728043 - Flags: approval-mozilla-b2g18? → approval-mozilla-b2g18+
(Assignee)

Comment 12

6 years ago
https://hg.mozilla.org/releases/mozilla-b2g18/rev/6759b57a7802
status-b2g18: affected → fixed
status-b2g18-v1.0.0: --- → wontfix
status-b2g18-v1.0.1: --- → wontfix
status-firefox20: --- → wontfix
status-firefox21: --- → wontfix
status-firefox22: --- → fixed
(Assignee)

Updated

6 years ago
Whiteboard: [target 3/22] → [NO_UPLIFT]
(Assignee)

Comment 13

6 years ago
Added [NO_UPLIFT] per recent commercial RIL compatibility issue. Waiting on further decision to keep the patch in b2g18 or to back it out.

------------------------------
If we really want to back them out, backing the following MMS bugs should be enough to make the commercial RIL compatible:

Bug 854422 - B2G MMS: should call .NotifyResponseTransaction() with MMS_PDU_STATUS_RETRIEVED after an MMS is retrieved under the RETRIEVAL_MODE_AUTOMATIC mode (a follow-up for bug 845643)
Bug 850680 - B2G MMS: broadcast "sms-received" and "sms-sent" system messages
Bug 850530 - B2G MMS: Use the same attribute name for delivery (s/state/delivery) like SMS
Bug 852911 - B2G MMS: fail to expose correct nsIDOMMozMmsMessage.attachments.
Bug 853725 - B2G MMS: fail to read nsIDOMMozMmsMessage.receivers for a received MMS (a follow-up of bug 849741).
Bug 853329 - B2G MMS: other Android phones cannot read attachments sent from FFOS
Bug 852471 - B2G MMS: provide nsIDOMMobileMessageManager interface (with sendMMS() first) (follow-up fix)
Bug 852460 - B2G MMS: provide nsIDOMMobileMessageManager.onreceived event (follow-up fix)
Bug 849741 - B2G MMS: provide nsIDOMMobileMessageManager.onreceived event
Bug 847756 - B2G MMS: provide nsIDOMMobileMessageManager.markMessageRead().
Bug 847736 - B2G MMS: provide nsIDOMMobileMessageManager.delete().
Bug 847738 - B2G MMS: provide nsIDOMMobileMessageManager.getMessage(). 
Bug 844431 - B2G MMS: provide nsIDOMMobileMessageManager interface (with sendMMS() first)
Bug 845643 - B2G MMS: Save retrieved MMS into database.
(Assignee)

Comment 14

6 years ago
Following the previous comment, some more needs to back out:

Bug 792321 - Check max values of MMS parameters in sendRequest.
Bug 833291 - B2G SMS & MMS: getMessages it's not working with PhoneNumberJS
Bug 844429 - B2G SMS & MMS: move SMS codes into dom/mobilemessage to make it generic for MMS
Bug 839436 - B2G MMS: make DB be able to save MMS messages.
(Assignee)

Comment 15

6 years ago
Confirming with Michael to see we should back out to Bug 839436 or just Bug 844431 (if we eventually decide to back out). Please see Bug 857632, comment #17.
(Assignee)

Comment 16

6 years ago
Per off-line discussion with Michael, we decided not to back out the MMS bugs that have already been in mozilla-b2g18. Removing [NO_UPLIFT] to make the check-in status sync'ed.
Whiteboard: [NO_UPLIFT]
You need to log in before you can comment on or make changes to this bug.