Closed Bug 1466884 Opened Last year Closed Last year

Add a line in the manifest for Samsung store

Categories

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

enhancement
Not set

Tracking

(firefox61 verified, firefox62 verified)

VERIFIED FIXED
mozilla62
Tracking Status
firefox61 --- verified
firefox62 --- verified

People

(Reporter: sylvestre, Assigned: petru)

References

Details

(Whiteboard: --do_not_change--[priority:high])

Attachments

(1 file)

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.
Whiteboard: --do_not_change--[priority:high]
Petru can you look at this ASAP? hopefully a quick fix.
Flags: needinfo?(petru.malek)
Flags: needinfo?(petru.malek) → needinfo?(petru.lingurar)
Assignee: nobody → petru.lingurar
Flags: needinfo?(petru.lingurar)
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!
Attachment #8983846 - Flags: review+
Attachment #8983846 - Flags: review?(sdaswani)
@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.
(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.
Pushed by nalexander@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/bf0127602279
Add permission needed to publish on Samsung AppStore; r=nalexander
https://hg.mozilla.org/mozilla-central/rev/bf0127602279
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → Firefox 62
Flags: qe-verify+
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?
The upload of nighty in Google play worked!
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]:
Attachment #8983846 - Flags: approval-mozilla-beta?
I might ask for an uplift in case we do a dot release for 60
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.
Attachment #8983846 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
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 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!
Attachment #8983846 - Flags: approval-mozilla-release?
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.
Attachment #8983846 - Flags: approval-mozilla-release? → approval-mozilla-release-
Status: RESOLVED → VERIFIED
See Also: → 1477177
Product: Firefox for Android → Firefox Build System
Target Milestone: Firefox 62 → mozilla62
You need to log in before you can comment on or make changes to this bug.