Skip to content

Commit

Permalink
android: Actually declare VIBRATE permission.
Browse files Browse the repository at this point in the history
The wix notification library's manifest file has this element with
`android:maxSdkVersion="18"`, so it only applies on API <= 18 (which
we don't even support).  That appears to be intended as a workaround
for an ancient Android bug, fixed in Android 4.2.1 and 4.3 here:

  https://android.googlesource.com/platform/frameworks/base/+/cc2e849fa

  Notification vibration improvements: [DO NOT MERGE]

   - When notifications vibrate as a fallback (that is,
     because they want to play a sound but the device is in
     vibrate mode), this no longer requires the VIBRATE
     permission.
   - As a bonus, if your notifications use DEFAULT_VIBRATE,
     you don't need the VIBRATE permission either.
   - If you specify a custom vibration pattern, you'll still
     need the VIBRATE permission for that.
   - Notifications vibrating in fallback mode use same
     vibration pattern but can be changed easily in future.
   - The DEFAULT_VIBRATE and fallback vibrate patterns are now
     specified in config.xml.

(That commit appears in `git log android-4.2_r1..android-4.2.1_r1`,
and is present in `android-4.3_r1` too.  Android 4.2 is API 17, and
Android 4.3 is API 18.)

OK, but we *do* specify a custom vibration pattern.  So we need the
VIBRATE permission even after this bug fix.

And we've had this element in our manifest to do that... but when the
Android build system merges our manifest with the library's, that
extra `android:maxSdkVersion` attribute from the library gets merged
into this element, so the much narrower workaround-request in the
library effectively clobbers our broader request.

As a result, our custom vibration pattern has basically never worked,
except for the few users on API <=18 back when we even supported that.
Fix the manifest so it can work.

It still doesn't work on API >=26, for the same reason as zulip#3150: we
need to specify the pattern on the channel, not the individual
notification.  We'll fix that separately.
  • Loading branch information
gnprice committed Dec 27, 2018
1 parent 4627366 commit 8e364be
Showing 1 changed file with 2 additions and 1 deletion.
3 changes: 2 additions & 1 deletion android/app/src/main/AndroidManifest.xml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@
<uses-permission android:name="${applicationId}.permission.C2D_MESSAGE" />
<uses-permission android:name="android.permission.INTERNET" />
<uses-permission android:name="android.permission.ACCESS_NETWORK_STATE" />
<uses-permission android:name="android.permission.VIBRATE"/>
<uses-permission android:name="android.permission.VIBRATE"
tools:node="replace"/>
<uses-permission android:name="android.permission.SYSTEM_ALERT_WINDOW"
tools:remove="${excludeSystemAlertWindowPermission}"/>
<uses-permission android:name="android.permission.READ_PHONE_STATE"
Expand Down

0 comments on commit 8e364be

Please sign in to comment.