B2G MMS: Use the same attribute name for delivery (s/state/delivery) like SMS

RESOLVED FIXED in Firefox 22

Status

()

RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: airpingu, Assigned: airpingu)

Tracking

({dev-doc-needed})

Trunk
mozilla22
ARM
Gonk (Firefox OS)
dev-doc-needed
Points:
---
Dependency tree / graph
Bug Flags:
in-moztrap -

Firefox Tracking Flags

(blocking-b2g:leo+, 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
Please see the IDL proposal at Bug 760065. We need to s/delivery/state in nsIDOMMozSmsMessage, which sounds a better name and can be consistent with nsIDOMMozMmsMessage.
Assignee: nobody → gene.lian
Whiteboard: [target 3/22]
(Assignee)

Comment 1

6 years ago
Currently, for reading a message's delivery state (ex, sent, sending, received... etc), we have to do something ugly like:

  if (message.type == "sms") {
    // read |message.delivery|
  } else if (message.type == "mms") {
    // read |message.state|
  }

because the new nsIDOMMozMmsMessage structure decided to use a better name |.state| instead of the nsIDOMMozSmsMessage's |.delivery|. If we also s/delivery/state in the nsIDOMMozSmsMessage, that will break the current Message App for reading SMS. However, the benefit is the Gaia codes don't need to consider different branches based on the message type.

Jonas and Mounir, may I have your preference regarding this issue? To do the renaming or not to do?
Flags: needinfo?(mounir)
Flags: needinfo?(jonas)
(Assignee)

Updated

6 years ago
Keywords: dev-doc-needed
make mms use the same name as SMS is for now
Flags: needinfo?(jonas)
Flags: needinfo?(mounir)
(Assignee)

Comment 3

6 years ago
Got it. Thanks Jonas!
Summary: B2G MMS: s/delivery/state in nsIDOMMozSmsMessage to be consistent with nsIDOMMozMmsMessage → B2G MMS: Use the same attribute name for delivery (s/state/delivery) like SMS
(Assignee)

Comment 4

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

Hi Jonas and Mounir,

This is a simple change. Let's use the same attribute name |.delivery| for MMS, which is compatible with SMS for now. Need super-review since it touches IDL. Thanks for your time.
Attachment #726483 - Flags: superreview?(jonas)
Attachment #726483 - Flags: review?(mounir)
(Assignee)

Comment 5

6 years ago
This is a follow-up fix for bug 844431. Let's mark leo+ for this also.
Blocks: 844431
blocking-b2g: --- → leo?
No longer depends on: 844431
Attachment #726483 - Flags: review?(mounir) → review+
Attachment #726483 - Flags: superreview?(jonas) → superreview+
Though in general you didn't need to make such a large change here. Simply changing the name of the API as exposed to JS would have been enough. I.e. a simple change in the .idl and the C++ functions would have been enough.
(Assignee)

Comment 7

6 years ago
No worries. It doesn't bother. Just hoping to make sure the terminologies consistent with SMS for now as possible as we can to prevent others from getting confused between |delivery| and |state|.

Will land this later.

Comment 9

6 years ago
https://hg.mozilla.org/mozilla-central/rev/279b55d18083
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla22

Updated

6 years ago
blocking-b2g: leo? → leo+
(Assignee)

Comment 11

6 years ago
Created attachment 730530 [details] [diff] [review]
[b2g18] Patch
Attachment #730496 - Attachment is obsolete: true
(Assignee)

Comment 12

6 years ago
https://hg.mozilla.org/releases/mozilla-b2g18/rev/da47922ec8e5
status-b2g18: --- → 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]

Updated

5 years ago
Flags: in-moztrap-
You need to log in before you can comment on or make changes to this bug.