Closed
Bug 810067
Opened 12 years ago
Closed 12 years ago
B2G MMS: support automatic/manual/never retrieval modes
Categories
(Core :: DOM: Device Interfaces, defect)
Tracking
()
People
(Reporter: vicamo, Assigned: ctai)
References
Details
(Keywords: dev-doc-needed)
Attachments
(1 file, 8 obsolete files)
(deleted),
patch
|
Details | Diff | Splinter Review |
MMS supports delayed retrieval. On receiving a M-Notification.ind, application should be able to determine what to do next according to either user settings or X-Mms-Recommended-Retrieval-Mode and X-Mms-Recommended-Retrieval-Mode-Text.
Reporter | ||
Updated•12 years ago
|
Depends on: message-database
Assignee | ||
Updated•12 years ago
|
QA Contact: ctai
Assignee | ||
Comment 1•12 years ago
|
||
We might need below task to do...
1. Hook settings to decide the retrieval mode(auto/auto-home/deferred/never)
2. We might need a transaction service to
1. Check MMS network availability
2. According to retrieval mode to decide when to download MMS entity.
3. Control the transaction state.
4. Keep pending/processing transaction in somewhere.
3. Need M-acknowledge.ind pdu for deferred mode.
4. Need Delivery Report related PDU
Assignee | ||
Comment 2•12 years ago
|
||
We might need to save the pending/processing transaction into database. And remove from database when finished.
Assignee | ||
Updated•12 years ago
|
QA Contact: ctai
Assignee | ||
Comment 3•12 years ago
|
||
After read more document, I think we don't need Delivery Report related PDU in MMS client. Because below reasons.
"A recipient MMS client can deny the generation of delivery reports (parameter X-Mms-Report-Allowed for M-acknowledge.ind and M-notifyresp.ind PDUs). If allowed by the recipient MMS client, the recipient MMSC generates the delivery report and forwards it to the originator MMSC over the MM4 interface. Upon receipt of the delivery report, the originator MMSC delivers it to the originator MMS client over the MM1 interface with the M-delivery.ind PDU"
Assignee | ||
Comment 4•12 years ago
|
||
But we might need read report and message forward related PDUs.
Updated•12 years ago
|
Assignee: nobody → ctai
Assignee | ||
Comment 5•12 years ago
|
||
Assignee | ||
Comment 6•12 years ago
|
||
Attachment #716969 -
Attachment is obsolete: true
Assignee | ||
Updated•12 years ago
|
Attachment #716972 -
Flags: feedback?(vyang)
Assignee | ||
Comment 7•12 years ago
|
||
Attachment #716972 -
Attachment is obsolete: true
Attachment #716972 -
Flags: feedback?(vyang)
Assignee | ||
Comment 8•12 years ago
|
||
Attachment #716984 -
Attachment is obsolete: true
Assignee | ||
Comment 9•12 years ago
|
||
Attachment #717702 -
Attachment is obsolete: true
Assignee | ||
Updated•12 years ago
|
Attachment #717704 -
Flags: review?(vyang)
Reporter | ||
Comment 10•12 years ago
|
||
Comment on attachment 717704 [details] [diff] [review]
Retrieval mode support
Review of attachment 717704 [details] [diff] [review]:
-----------------------------------------------------------------
::: dom/mms/src/ril/MmsService.js
@@ +35,5 @@
> const TIME_TO_RELEASE_MMS_CONNECTION = 30000;
>
> +const MANUAL_RETRIEVAL_MODE = "manual";
> +const AUTOMATIC_RETRIEVAL_MODE = "automatic";
> +const NEVER_RETRIEVAL_MODE = "never";
|const RETRIEVAL_MODE_MANUAL = "manual";| instead.
@@ +745,5 @@
> * and M-Acknowledge.ind PDU.
> */
> confSendDeliveryReport: CONFIG_SEND_REPORT_DEFAULT_YES,
>
> + retrievalModePerferenceName: 'dom.mms.retrieval_mode',
|const PERF_RETRIEVAL_MODE = "dom.mms.retrieval_mode";| before RETRIEVAL_MODE_FOO instead.
@@ +836,5 @@
> + new NotifyResponseTransaction(transactionId, mmsStatus, reportAllowed);
> + transaction.run();
> + }).bind(this));
> + return;
> + } else if (NEVER_RETRIEVAL_MODE === retrievalMode) {
You don't need |else| after a return statement.
@@ +843,5 @@
> + let transactionId = notification.headers["x-mms-transaction-id"];
> +
> + let transaction = new NotifyResponseTransaction(transactionId,
> + MMS.MMS_PDU_STATUS_REJECTED,
> + false);
Should calculate reportAllowed here based only on notification.headers["x-mms-delivery-report"].
@@ +846,5 @@
> + MMS.MMS_PDU_STATUS_REJECTED,
> + false);
> + transaction.run();
> + return;
> + } else { //MANUAL_RETRIEVAL_MODE and wrong retrievalMode value
|else|
Attachment #717704 -
Flags: review?(vyang)
Comment 11•12 years ago
|
||
Comment on attachment 717704 [details] [diff] [review]
Retrieval mode support
Review of attachment 717704 [details] [diff] [review]:
-----------------------------------------------------------------
::: dom/mms/src/ril/MmsService.js
@@ +707,5 @@
> + // Optional fields
> + headers["x-mms-report-allowed"] = reportAllowed;
> +
> + this.istream = MMS.PduHelper.compose(null, {headers: headers});
> +}
Nit: add a blank line
@@ +745,5 @@
> * and M-Acknowledge.ind PDU.
> */
> confSendDeliveryReport: CONFIG_SEND_REPORT_DEFAULT_YES,
>
> + retrievalModePerferenceName: 'dom.mms.retrieval_mode',
Nit: s/retrievalModePerferenceName/retrievalModePreferenceName
@@ +809,5 @@
> // notification
> +
> + let retrievalMode = MANUAL_RETRIEVAL_MODE;
> + try {
> + retrievalMode = Services.prefs.getCharPref(this.retrievalModePerferenceName);
Nit: s/retrievalModePerferenceName/retrievalModePreferenceName
@@ +813,5 @@
> + retrievalMode = Services.prefs.getCharPref(this.retrievalModePerferenceName);
> + } catch (e) {}
> +
> + if (AUTOMATIC_RETRIEVAL_MODE === retrievalMode) {
> + this.retrieveMessage(url, (function (mmsStatus, retrievedMsg) {
Please name the anonymous function
Assignee | ||
Comment 12•12 years ago
|
||
Attachment #717704 -
Attachment is obsolete: true
Assignee | ||
Updated•12 years ago
|
Attachment #717712 -
Flags: review?(vyang)
Reporter | ||
Comment 13•12 years ago
|
||
Comment on attachment 717712 [details] [diff] [review]
Retrieval mode supported
Review of attachment 717712 [details] [diff] [review]:
-----------------------------------------------------------------
Thank you :)
::: dom/mms/src/ril/MmsService.js
@@ +848,2 @@
>
> let transactionId = notification.headers["x-mms-transaction-id"];
Please move this outside if-block and share with manual mode. Actually you you can share everything by:
let mmsStatus = RETRIEVAL_MODE_NEVER === retrievalMode
? MMS.MMS_PDU_STATUS_REJECTED : MMS.MMS_PDU_STATUS_DEFERRED;
Attachment #717712 -
Flags: review?(vyang) → review+
Assignee | ||
Comment 14•12 years ago
|
||
Attachment #717712 -
Attachment is obsolete: true
Assignee | ||
Updated•12 years ago
|
Keywords: checkin-needed
Comment 15•12 years ago
|
||
Comment on attachment 717734 [details] [diff] [review]
Retrieval mode supported
Review of attachment 717734 [details] [diff] [review]:
-----------------------------------------------------------------
::: dom/mms/src/ril/MmsService.js
@@ +34,5 @@
> const TIME_TO_BUFFER_MMS_REQUESTS = 30000;
> const TIME_TO_RELEASE_MMS_CONNECTION = 30000;
>
> +
> +const PERF_RETRIEVAL_MODE = 'dom.mms.retrieval_mode';
Nit: PREF_RETRIEVAL_MODE
@@ +810,4 @@
>
> + let retrievalMode = RETRIEVAL_MODE_MANUAL;
> + try {
> + retrievalMode = Services.prefs.getCharPref(PERF_RETRIEVAL_MODE);
Nit: PREF_RETRIEVAL_MODE
Assignee | ||
Updated•12 years ago
|
Keywords: checkin-needed
Assignee | ||
Comment 16•12 years ago
|
||
Attachment #717734 -
Attachment is obsolete: true
Assignee | ||
Comment 17•12 years ago
|
||
Attachment #717736 -
Attachment is obsolete: true
Assignee | ||
Updated•12 years ago
|
Keywords: checkin-needed
Assignee | ||
Comment 18•12 years ago
|
||
Fixed.
Thanks for your comment.
(In reply to Fabrice Desré [:fabrice] from comment #15)
> Comment on attachment 717734 [details] [diff] [review]
> Retrieval mode supported
>
> Review of attachment 717734 [details] [diff] [review]:
> -----------------------------------------------------------------
>
> ::: dom/mms/src/ril/MmsService.js
> @@ +34,5 @@
> > const TIME_TO_BUFFER_MMS_REQUESTS = 30000;
> > const TIME_TO_RELEASE_MMS_CONNECTION = 30000;
> >
> > +
> > +const PERF_RETRIEVAL_MODE = 'dom.mms.retrieval_mode';
>
> Nit: PREF_RETRIEVAL_MODE
>
> @@ +810,4 @@
> >
> > + let retrievalMode = RETRIEVAL_MODE_MANUAL;
> > + try {
> > + retrievalMode = Services.prefs.getCharPref(PERF_RETRIEVAL_MODE);
>
> Nit: PREF_RETRIEVAL_MODE
Reporter | ||
Comment 19•12 years ago
|
||
Reporter | ||
Updated•12 years ago
|
Keywords: checkin-needed → dev-doc-needed
Comment 21•12 years ago
|
||
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
Comment 22•12 years ago
|
||
This doesn't apply cleanly to b2g18. Please either post a branch-specific patch suitable for landing or request approval for any bugs that this depends on.
Reporter | ||
Comment 23•12 years ago
|
||
(In reply to Ryan VanderMeulen [:RyanVM] from comment #22)
> This doesn't apply cleanly to b2g18. Please either post a
> branch-specific patch suitable for landing or request approval
> for any bugs that this depends on.
leo? for bug 833697 first, thank you :)
Comment 24•12 years ago
|
||
status-b2g18:
--- → fixed
status-b2g18-v1.0.0:
--- → wontfix
status-b2g18-v1.0.1:
--- → wontfix
status-firefox20:
--- → wontfix
status-firefox21:
--- → wontfix
status-firefox22:
--- → fixed
Updated•12 years ago
|
Blocks: message-database
No longer depends on: message-database
Updated•12 years ago
|
Flags: in-moztrap-
You need to log in
before you can comment on or make changes to this bug.
Description
•