Closed
Bug 1212679
Opened 9 years ago
Closed 9 years ago
Make sure we handle being the default SMS app correctly
Categories
(B2GDroid Graveyard :: General, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: reuben, Assigned: reuben)
References
Details
Attachments
(2 files, 2 obsolete files)
(deleted),
patch
|
fabrice
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
fabrice
:
review+
snorp
:
review+
|
Details | Diff | Splinter Review |
KK makes all the Telephony stuff read-only unless you're the default SMS app on the system. This bug is specifically to make sure everything works when we are the default app.
Assignee | ||
Comment 1•9 years ago
|
||
Can we just import this code like this? Is the Apache license a problem at all?
Attachment #8683790 -
Flags: feedback?(fabrice)
Assignee | ||
Comment 2•9 years ago
|
||
Sorry, I hit submit too early. We need those definitions to be able to parse downloaded MMS's, they're not exported so the only way to get them is by copying from the Android source code.
Code was imported from https://android.googlesource.com/platform/frameworks/opt/telephony/
Assignee | ||
Comment 3•9 years ago
|
||
SMS works, MMS still needs work. SMS is hard to test locally (you can do it on the emulator[0]), MMS is not testable at all, you need to send a real MMS from another phone to be able to test your code :(
Not to mention that this stuff is barely documented. The good references are the MMS demo[1] and the Telephony source code[2].
[0] http://developer.android.com/tools/devices/emulator.html#sms
[1] https://github.com/android/platform_development/tree/master/samples/ApiDemos/src/com/example/android/apis/os
[2] http://grepcode.com/file/repository.grepcode.com/java/ext/com.google.android/android/5.1.1_r1/android/provider/Telephony.java
Comment 4•9 years ago
|
||
Comment on attachment 8683790 [details] [diff] [review]
Import MMS PDU definitions and helpers from Android
Review of attachment 8683790 [details] [diff] [review]:
-----------------------------------------------------------------
I defer to Gerv for the license aspect.
Reuben, is this code working on all releases >= KK ?
Attachment #8683790 -
Flags: feedback?(fabrice) → feedback?(gerv)
Comment 5•9 years ago
|
||
Importing Apache-licensed code as whole files is fine - no licensing actions to be taken.
Gerv
Assignee | ||
Comment 6•9 years ago
|
||
(In reply to [:fabrice] Fabrice Desré from comment #4)
> Reuben, is this code working on all releases >= KK ?
Yep, all of this stuff was introduced with KK and hasn't changed. I just need to make sure I import the KK version of those MMS files.
Updated•9 years ago
|
Attachment #8683790 -
Flags: feedback?(gerv) → feedback+
Assignee | ||
Comment 8•9 years ago
|
||
This is the supporting code needed to save incoming WAP messages into the Android content provider, minus the DRM stuff which requires access to (even more) Android internals.
Attachment #8683790 -
Attachment is obsolete: true
Attachment #8695670 -
Flags: review?(fabrice)
Assignee | ||
Comment 9•9 years ago
|
||
With this patch b2gdroid can be set as the default messaging app and handle the relevant intents. The "default messaging app" API (if you can even call it that) is incredibly poorly designed and basically requires copying the code from Android.
Basically there's a few things you need to handle as the default app:
- Different intents for receiving messages. These are called *_DELIVER (vs RECEIVED), and they're only sent to the default app. When you're the default app you're required to persist incoming messages.
- Intents for other apps to trigger the SMS compose UI. This is done by creating an equivalent web activity on the Gecko side.
The extra code needed to persist SMS messages is in GeckoSmsManager.java. The code to persist MMS messages is added to b2gdroid only. Most of the MMS stuff is adapted from the Android telephony framework: https://android.googlesource.com/platform/frameworks/opt/telephony
Attachment #8683793 -
Attachment is obsolete: true
Attachment #8695672 -
Flags: review?(fabrice)
Assignee | ||
Comment 10•9 years ago
|
||
Comment on attachment 8695672 [details] [diff] [review]
Handle default SMS/MMS intents
James, can you review these changes, in particular the mobile/android/base/ stuff?
Attachment #8695672 -
Flags: review?(snorp)
Assignee | ||
Comment 11•9 years ago
|
||
Oh, and if we ever bump the minimum compatibility to Lollipop we can get rid of a lot of this code, since they added an auto-persist option to SmsManager that takes care of most of the boring work, and lets apps focus on their actual features.
Attachment #8695672 -
Flags: review?(snorp) → review+
Comment 12•9 years ago
|
||
Comment on attachment 8695672 [details] [diff] [review]
Handle default SMS/MMS intents
Review of attachment 8695672 [details] [diff] [review]:
-----------------------------------------------------------------
::: mobile/android/b2gdroid/app/src/main/AndroidManifest.xml
@@ +72,5 @@
>
> <meta-data android:name="com.sec.android.support.multiwindow" android:value="true"/>
>
> + <!-- Listen for incoming SMS messages -->
> + <receiver android:name="org.mozilla.gecko.GeckoSmsManager"
nit: ".GeckoSmsManager" ?
@@ +119,5 @@
> </intent-filter>
>
> + <!-- Handle SMS intents -->
> + <intent-filter>
> + <action android:name="android.intent.action.SEND" />
nit: trailing whitespace
::: mobile/android/b2gdroid/app/src/main/java/org/mozilla/b2gdroid/Launcher.java
@@ +275,5 @@
> + JSONObject obj = new JSONObject();
> + try {
> + obj.put("action", "send_sms");
> + if (Intent.ACTION_SENDTO.equals(action)) {
> + String number = intent.getData().getPath();
nit: final String (or inline in obj.put)
@@ +278,5 @@
> + if (Intent.ACTION_SENDTO.equals(action)) {
> + String number = intent.getData().getPath();
> + obj.put("number", number);
> + }
> + String body = intent.getStringExtra("sms_body");
same here.
::: mobile/android/b2gdroid/app/src/main/java/org/mozilla/b2gdroid/MmsService.java
@@ +14,5 @@
> +import android.net.Uri;
> +import android.os.Build;
> +import android.provider.Telephony;
> +import android.telephony.SmsManager;
> +import android.telephony.TelephonyManager;
that seems unused.
Attachment #8695672 -
Flags: review?(fabrice) → review+
Updated•9 years ago
|
Attachment #8695670 -
Flags: review?(fabrice) → review+
Assignee | ||
Comment 13•9 years ago
|
||
(In reply to [:fabrice] Fabrice Desré from comment #12)
> ::: mobile/android/b2gdroid/app/src/main/AndroidManifest.xml
> @@ +72,5 @@
> >
> > <meta-data android:name="com.sec.android.support.multiwindow" android:value="true"/>
> >
> > + <!-- Listen for incoming SMS messages -->
> > + <receiver android:name="org.mozilla.gecko.GeckoSmsManager"
>
> nit: ".GeckoSmsManager" ?
The "package" property of the manifest is "org.mozilla.b2gdroid", so that would resolve to org.mozilla.b2gdroid.GeckoSmsManager.
I've addressed the other comments.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=29e263016b67
Comment 14•9 years ago
|
||
Comment 15•9 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/219dee692172
https://hg.mozilla.org/mozilla-central/rev/9a03868366df
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•