Julien Wajsberg [:julienw] Reporter
	Description • 11 years ago

STR:
* set "MMS Automatic retrieval" to off
* receive a MMS with attachment
* enable airplane mode
* try to download the MMS

Expected:
* we should see an error related to airplane mode

Actual:
* no error is displayed, only the error icon is added.

Note that I didn't find any error message related to airplane mode for this case (we have one for _sending_ messages but it's not suited for this bug).

NI ayman for providing more information.
QAwanted to know the behavior in v1.1.

	Julien Wajsberg [:julienw] Reporter
	Comment 1 • 11 years ago

Actual in 1.4:
* the "missing sim card" error is displayed.

	Julien Wajsberg [:julienw] Reporter
	Comment 2 • 11 years ago

Related to Bug 981077.

When sending, we get a "RadioDisabledError" error from Gecko. Maybe we should have the same one here?

(and in that case we'll need Gaia change because we'll need to distinguish sending errors from receiving errors).

	Bevis Tseng[:bevis][:btseng](Exited) Assignee
	Comment 3 • 11 years ago

Yes, for retrieving, we should check airplane mode settings with higher priority than checking iccid.

	Jason Smith [:jsmith]
	Comment 5 • 11 years ago

Minor bug, so there's no value to pursuing a branch comparison here.

	Bevis Tseng[:bevis][:btseng](Exited) Assignee
	Comment 6 • 11 years ago

1. Move internally utilities to the same code segment.
2. Wrap fetching ril.radio.disabled into new function of getRadioDisabledState().

	Bevis Tseng[:bevis][:btseng](Exited) Assignee
	Comment 8 • 11 years ago

1. Move redundant code into head.js.
2. Add new test case to verify error code of retrieving while airplane mode is on.

	Bevis Tseng[:bevis][:btseng](Exited) Assignee
	Comment 9 • 11 years ago

I'd like to run this test in chrome process for more flexible content creation of MMS notification that could also be used in bug 981077.

	Vicamo Yang [:vicamo][:vyang]
	Comment 10 • 11 years ago

Comment on attachment 8396172 [details] [diff] [review]
Patch Part3 v2: Add Test case to verify RadioDisabledError. r=vyang.

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

::: dom/mobilemessage/tests/marionette/head.js
@@ +372,5 @@
>    return runEmulatorCmdSafe(command);
>  }
>  
>  /**
> + * Send raw SMS TPDU to emulator and wait

nit: 'multiple raw ...'

@@ +387,5 @@
> + *   result -- an array of emulator response lines.
> + *
> + * @return A deferred promise.
> + */
> +function sendRawSmsAndWait(aPdus) {

nit: please rename to `sendMultipleRawSmsToEmulatorAndWait`.

::: dom/mobilemessage/tests/marionette/test_error_of_mms_manual_retrieval.js
@@ +4,5 @@
> +MARIONETTE_TIMEOUT = 60000;
> +// We apply "chrome" context to be more flexible to
> +// specify the content of M-Notification.ind such as iccId
> +// for different kinds of testing.
> +MARIONETTE_CONTEXT = "chrome";

I think we can reuse the |buildBinaryPdus| function you had in “test_mt_sms_concatenation.js” to construct an MMS notification indication PDU.  This way, we don't have to run this script in chrome context and can reuse all the utility functions in "head.js".

::: dom/mobilemessage/tests/marionette/test_replace_short_message_type.js
@@ +38,5 @@
>    let pdu = buildPdu(aSender, aPid, PDU_UD_B);
>    ok(true, "Sending " + pdu);
>  
> +  return sendRawSmsAndWait([pdu])
> +    .then((results) => function(results) {

function(results) {

@@ +59,5 @@
>  function testPid(aPid) {
>    log("Test message PID '" + aPid + "'");
>  
> +  return sendRawSmsAndWait([buildPdu(PDU_SENDER_0, aPid, PDU_UD_A)])
> +    .then((results) => function(results) {

ditto.

@@ +67,5 @@
>        for (let pid of PDU_PIDS) {
>          let verify = (aPid !== PDU_PID_NORMAL && pid === aPid)
>                     ? verifyReplaced : verifyNotReplaced;
>          promise =
> +          promise.then(verify.bind(null, receivedMsg, PDU_SENDER_0, pid))

() => verify(receivedMsg, PDU_SENDER_0, pid)

@@ +73,1 @@
>                                                PDU_SENDER_1, pid));

() => verifyNotReplaced(receivedMsg, PDU_SENDER_1, pid)

	Vicamo Yang [:vicamo][:vyang]
	Comment 11 • 11 years ago

(In reply to Bevis Tseng [:bevistseng] (btseng@mozilla.com) from comment #9)
> I'd like to run this test in chrome process for more flexible content
> creation of MMS notification that could also be used in bug 981077.

That's not really a big problem.  Your previous revision is better for me.

	Vicamo Yang [:vicamo][:vyang]
	Comment 12 • 11 years ago

Comment on attachment 8396172 [details] [diff] [review]
Patch Part3 v2: Add Test case to verify RadioDisabledError. r=vyang.

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

Per offline discuss, routing mms notification indication through emulator console will always get an binary message that contains a correct ICCID.  Unless we supports changing ICCID at runtime, this is really a big problem for testing bug 981077.  So, thank you, Bevis.

::: dom/mobilemessage/tests/marionette/test_error_of_mms_manual_retrieval.js
@@ +116,5 @@
> +    Services.prefs.setBoolPref("ril.radio.disabled", aDisabled);
> +};
> +
> +Promise.resolve()
> +  .then(() => testRetrieve(Ci.nsIMobileMessageCallback.RADIO_DISABLED_ERROR,

You may just have:

  testRetrieve(...)
    .then(...);

@@ +118,5 @@
> +
> +Promise.resolve()
> +  .then(() => testRetrieve(Ci.nsIMobileMessageCallback.RADIO_DISABLED_ERROR,
> +        setRadioDisabled.bind(null, true),
> +        setRadioDisabled.bind(null, false)))

nit: align to |Ci.foo|

	Bevis Tseng[:bevis][:btseng](Exited) Assignee
	Comment 13 • 11 years ago

address comment 11 and comment 12.

	Vicamo Yang [:vicamo][:vyang]
	Comment 14 • 11 years ago

Comment on attachment 8396960 [details] [diff] [review]
Patch Part3 v3: Add Test case to verify RadioDisabledError. r=vyang.

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

Thank you :)

	Bevis Tseng[:bevis][:btseng](Exited) Assignee
	Comment 15 • 11 years ago

try server result is green:
https://tbpl.mozilla.org/?tree=Try&rev=981e7f548e3b

	Ryan VanderMeulen [:RyanVM]
	Comment 16 • 11 years ago

https://hg.mozilla.org/integration/b2g-inbound/rev/f12296b754c7
https://hg.mozilla.org/integration/b2g-inbound/rev/7d665a2dab28
https://hg.mozilla.org/integration/b2g-inbound/rev/d7f03eb86cdb

	Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)
	Comment 17 • 11 years ago

https://hg.mozilla.org/mozilla-central/rev/f12296b754c7
https://hg.mozilla.org/mozilla-central/rev/7d665a2dab28
https://hg.mozilla.org/mozilla-central/rev/d7f03eb86cdb

	Bevis Tseng[:bevis][:btseng](Exited) Assignee
	Comment 18 • 11 years ago

Hi Julien,

Please be informed to have the corresponding warning message to be displayed for 
|retrieving| when airplane mode is on. :)

	Julien Wajsberg [:julienw] Reporter
	Comment 19 • 11 years ago

Filed bug 990021