STR: * turn on flight mode * send a SMS to a new number Expected: * the message is in error and the "flight mode" alert dialog should be displayed. Actual: * the messages is in error, but the "flight mode" alert dialog is not displayed. Note that it is when you send a SMS to a number that already has a thread. I think this is a regression, but I'm not sure. The bug happens in v1.1 for sure.

On the first try to send an SMS in airplane mode we receive an error: at app://sms.gaiamobile.org/js/message_manager.js:339 in onerror: Reading the database. Error: InternalError and this._mozMobileMessage never fires 'failed' event. Looks like a platform issue.

(I know I've seen the same error in another bug but I can't find it)

(In reply to Julien Wajsberg [:julienw] from comment #2) > (I know I've seen the same error in another bug but I can't find it) Do you mean bug 874186? However bug 874186 requires we have no thread for the recipient yet.

Vicamo> this bug needs a new recipient as well :) Yep, that was bug 874186, but here we still reproduce it afaik.

Just tried it again with a recent b2g18: * we don't have the logcat error anymore * but we still have the bug Michal, could you check with a newer b2g18 if we get the 'failed' event ?

Nothing changed - the bug still occurs, the event doesn't fire and the error is still there SOMETIMES.

Comment on attachment 755612 [details] patch reviewed on github, mostly nits

Before merging, I would like to confirm with Ayman. @Ayman: Do we need this alert in airplane mode? Thanks!

If we don't need the alert anymore, I think part of the patch is still necessary: we can't add the class on a dom node which doesn't exist :)

To be very clear, we have this alert at least since I started to work on the Sms app :) I'd be happy to remove this code (I love removing code ;) ) but this is here since ages. It was failing in this specific case only.

(In reply to Borja Salguero [:borjasalguero] from comment #9) > Before merging, I would like to confirm with Ayman. > > @Ayman: Do we need this alert in airplane mode? Thanks! Hey borja, yes we do need this alert... original plan was to provide the end user a direct rout from the alter to turn flight mode off... but that apparently is technically not possible at the moment.. but we still keep the alert and improve its functionality when we can.

...in my personal opinion, the alerts articulation is a little clumsy. We dont need to block the user from continuing on their path by using a full screen message to inform them that they are in flight mode they must cancel in order to proceed. We do enough notification within in the message thread itself with the error state of the message module itself. We should have a temporary message that informs but does not intrude like what we display when the users moves between SMS packets... However the full screen message is in line with building blocks, and we were blocked from implementing a temporary message at the time the flight mode message was added because there was no such building block.

I agree that the full screen message is most useful if we can do some action on it. Maybe we should check for the flight mode setting earlier and display a banner message asap in the Sms app. But let's do that in another bug if you will ?

> But let's do that in another bug if you will ? Totally! ...comment 13 was just a stream of consciousness as i was looking at the UX ;)

Comment on attachment 755612 [details] patch r=me cheers !

Thanks! Merged: https://github.com/mozilla-b2g/gaia/commit/2c7fc2127478cd881f3a9f948dfa02d0f7684d29

If this is intented for uplift to v1.1 please nominate and fill out the approval form with risk evaluation, testing done, regressing bug, etc.

It's strange, I am not reproducing the bug in v1-train but the patch is not uplifted in that branch. Requesting QA to confirm it.

Unable to repro issue on Unagi Build ID: 20130605070207 Gecko: http://hg.mozilla.org/releases/mozilla-b2g18/rev/09dc1ae3b1b5 Gaia: 92a6e36957145cdb2ac8867e5cdba8ecf12308fc When sending a sms message with airplane mode enabled message is now displayed saying 'Airplane Mode Activated: To send a message; first disable airplane mode"

Unagi Build ID:Gecko-363855a.Gaia-92a6e36 (20130605180132) Gaia: 92a6e36957145cdb2ac8867e5cdba8ecf12308fc STEPS (Fisrt SMS) 1.Turn on flight mode 2.Send a SMS to a new number (not in contacts). It must be the first SMS, there is no previous thread. Expected: * The message is in error and the "flight mode" alert dialog screen should be displayed. Actual: * The SMS is in error, but the "flight mode" alert dialog screen is not displayed. No message informs about what's happening. Note that first time I tried it, an alert message "Airplane mode on" appeared in the thread view (see attachment 2013 [details]-06-05-20-57-34), but did not appear in the following "First SMS" tests (see attachment 2013 [details]-06-05-21-38-52), . Then... STEPS (Existing SMS thread) 1.Turn on flight mode 2.Send a SMS to a new number (not in contacts)that already has a SMS thread. Expected: * The message is in error and the "flight mode" alert dialog screen should be displayed. Actual: * The SMS is in error, and the "flight mode" alert dialog is displayed showing 'Airplane Mode Activated: To send a message; first disable airplane mode".

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 #): User impact if declined: The user doesn't receive a warning when trying to send a SMS on airplane mode to a new number Testing completed: Risk to taking this patch (and alternatives if risky): In my opinion, risk is low-medium, as we mess with the thread ui and warning messages, but I'd like to ask Michael for his opinion, as he made the code changes and knows it better String or UUID changes made by this patch:

this is blocking a blocker from merging clean so I'm picking it onto the train: uplifted master: 747f412b370039569e37161e43185427ad20fbb8 to v1-train: 8eb11d8de3fa994e6db893a688df4b4d1dabb66d

Cleaning flags as it's solved and uplifted

Comment on attachment 759026 [details] Approval request This has been already uplifted

Note for next time: Corey, you're supposed to wait for approval before uplifting, even if it's blocking another uplift. Also, having conflicts is not a good reason to uplift, as you can resolve conflict by hand. (but if you're confident that the conflicting bug will get approved, then you can just wait of course).