1466884 - Add a line in the manifest for Samsung store

Status: VERIFIED FIXED

(Firefox Build System :: Android Studio and Gradle Integration, enhancement)

Description

[Sylvestre Ledru [:Sylvestre]]

Seems that we need the following:

   <uses-permission android:name=”com.samsung.android.providers.context.permission.WRITE_USE_APP_FEATURE_SURVEY”/>

See https://stackoverflow.com/questions/25068907/http-seller-samsungapps-com-choosing-a-category

Otherwise, we cannot publish fennec on it.

Comment 1

[:sdaswani]

Petru can you look at this ASAP? hopefully a quick fix.

Comment 2

[Petru-Mugurel Lingurar [:petru]]

Attachment: Bug 1466884 - Add permission needed to publish on Samsung AppStore;

Review commit: https://reviewboard.mozilla.org/r/249690/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/249690/

Comment 3

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8983846 [details]
Bug 1466884 - Add permission needed to publish on Samsung AppStore;

https://reviewboard.mozilla.org/r/249690/#review255878

Technically, this looks fine.  For your future planning, I'd prefer if we _collapsed_ all these included manifests into `AndroidManifest.xml.in` -- they were just for GeckoView, and GV does things differently now, so we don't need the splits in this way anymore.

I can't find a reference stating that an unknown permission in a manifest is ignored.  Can you find such a reference?  Otherwise, I'll take a green try build and an install onto a phone that doesn't have the Samsung AppStore.

Thanks, Petru!

Comment 4

[Petru-Mugurel Lingurar [:petru]]

@Nick

I don't think an unknown permission is ignored in that it is skipped when building and parsing the manifest. It should always be included in the compiled package and would just produce warnings/errors when trying to use it. Couldn't find an official reference for this though.

I've made a full build locally (how do you get the apk from try?), analyzed the apk to make sure the new permission is in the manifest and installed it on 3 phones:
- Nexus 6P w Android 8.0 - app installed, worked normally, no warnings
- Samsung S7 w Android 7.0 - app installed, worked normally, no warnings
- Samsung J3 w Android 5.1.1 (without the new runtime permissions system) - app installed but immediately after the install process finished I would see in logs:
> W/PackageManager: Unknown permission com.samsung.android.providers.context.permission.WRITE_USE_APP_FEATURE_SURVEY in package org.mozilla.fennec_petrulingurar
Other than that the install process went fine and the app ran normally.

Comment 5

[Nick Alexander :nalexander [he/him]]

(In reply to Petru-Mugurel Lingurar[:petru] from comment #4)
> @Nick
> 
> I don't think an unknown permission is ignored in that it is skipped when
> building and parsing the manifest.

Sorry, I meant _ignored at runtime_.

 It should always be included in the
> compiled package and would just produce warnings/errors when trying to use
> it. Couldn't find an official reference for this though.

Right, I couldn't find this documented.

> I've made a full build locally (how do you get the apk from try?), analyzed
> the apk to make sure the new permission is in the manifest and installed it
> on 3 phones:
> - Nexus 6P w Android 8.0 - app installed, worked normally, no warnings
> - Samsung S7 w Android 7.0 - app installed, worked normally, no warnings
> - Samsung J3 w Android 5.1.1 (without the new runtime permissions system) -
> app installed but immediately after the install process finished I would see
> in logs:
> > W/PackageManager: Unknown permission com.samsung.android.providers.context.permission.WRITE_USE_APP_FEATURE_SURVEY in package org.mozilla.fennec_petrulingurar
> Other than that the install process went fine and the app ran normally.

This is enough for me.  I'll land your patch.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/bf0127602279
Add permission needed to publish on Samsung AppStore; r=nalexander

Comment 7

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/bf0127602279

Comment 8

[Petru-Mugurel Lingurar [:petru]]

Can this be checked and confirmed in that the apps continue to work normally on Samsung and non-Samsung devices with Android 6+ and also older Android versions?

Comment 9

[Sylvestre Ledru [:Sylvestre]]

The upload of nighty in Google play worked!

Comment 10

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8983846 [details]
Bug 1466884 - Add permission needed to publish on Samsung AppStore;

Approval Request Comment
[Feature/Bug causing the regression]: NA
[User impact if declined]: Not being able to upload the apk on Samsung store
[Is this code covered by automated tests?]: Yes (we do automatic upload on Google play)
[Has the fix been verified in Nightly?]: Yes, the last upload worked
[Needs manual test from QE? If yes, steps to reproduce]: Not necessary
[List of other uplifts needed for the feature/fix]: No
[Is the change risky?]: I don't think it is 
[Why is the change risky/not risky?]: Some third party stores might complain about a useless manifest but I don't expect they would reject the upload.
[String changes made/needed]:

Comment 11

[Sylvestre Ledru [:Sylvestre]]

I might ask for an uplift in case we do a dot release for 60

Comment 12

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 8983846 [details]
Bug 1466884 - Add permission needed to publish on Samsung AppStore;

Allows us to upload Fennec to the Samsung store. Approved for 61.0b13.

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/534148c94be1

Comment 14

[Andrei Bodea[:andrei]]

Hello,

No problems encountered today while installing and running the Beta 61.0b13 build on several Samsung devices and others:

Samsung Galaxy J7 Android 6.0.1
Samsung Galaxy S7 Android 8.0.0
Samsung Galaxy S7 Android 7.0.1
Samsung Galaxy S6 Android 6.0.1
Samsung Galaxy S5 Android 6.0.1
Samsung Galaxy J7 Android 6.0.1
Sony Xperia Z2    Android 6.0.1
Google Pixel C    Android 8.0.0
Xiaomi Redmi Note 3 Android 6.0.1
Samsung Galaxy Tab 3 Android 7.0

Thank you,
Andrei

Comment 15

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8983846 [details]
Bug 1466884 - Add permission needed to publish on Samsung AppStore;

Thanks for the verification!

In case we do a dot release for 60, I would like to get this patch but no need to do a dot release just for this patch.
Uplift request is in comment #10!

Comment 16

[Julien Cristau [:jcristau]]

Comment on attachment 8983846 [details]
Bug 1466884 - Add permission needed to publish on Samsung AppStore;

this will ship in 61, no more planned dot releases for fennec 60.