1390261 - Sometimes Firefox would require some permissions and don't ask for them

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Clément Lefèvre]

For example on a picture uploading platform, Firefox didn't had the storage permission, and when using the filepicker to upload a file, there was not a single time the permissions popup that appeared requiring the permission, leading to silent fail of the upload: Firefox doesn't have the permission to access to the file and never asked for it.

Problem was for example experienced on send.firefox.com, lut.im or framapic.org

Comment 1

[James Willcox (:snorp) ([e-mail]) (he/him)]

Dylan can you take a look? I helped him debug on IRC and I think a normal user would have zero chance of figuring out what went wrong here.

Comment 2

[Nevin Chen(Not active on Bugzilla)]

These seems to <input type="file" accept="....> so may be caused by my bug 1337692

Comment 3

[Nevin Chen(Not active on Bugzilla)]

I guess the promblem is I can only get MIME type information here. 
http://searchfox.org/mozilla-central/source/mobile/android/base/java/org/mozilla/gecko/FilePicker.java#

May I know how to get accept="image/*" information?

Comment 4

[Dylan Roeh (:droeh) (he/him)]

Attachment: Add external storage permissions for camera/microphone usage

It seems like there might be two issues here. I can't reproduce the problem solely with filepicker -- on all three sites I can successfully choose and upload photos without external storage permissions (which seems weird in and of itself...). However, when I try to upload directly from the camera, it fails silently as reported -- this patch fixes that issue.

If there's still an issue stopping upload from the filepicker I think more information is needed: what device is being used and what android version is installed, to start with.

Comment 5

[Clément Lefèvre]

(In reply to Dylan Roeh (:droeh) from comment #4)
> Created attachment 8903599 [details] [diff] [review]
> Add external storage permissions for camera/microphone usage
> 
> It seems like there might be two issues here. I can't reproduce the problem
> solely with filepicker -- on all three sites I can successfully choose and
> upload photos without external storage permissions (which seems weird in and
> of itself...). However, when I try to upload directly from the camera, it
> fails silently as reported -- this patch fixes that issue.
> 
> If there's still an issue stopping upload from the filepicker I think more
> information is needed: what device is being used and what android version is
> installed, to start with.

So, I'm using a Sony Z3 compact with base Sony image on Android 6.0.1

after further testing:

If I remove all permissions from Nightly and try to send files on send.firefox.com or lut.im I could notice that:
- Before asking what to use to send a file, Nightly still requires microphone and camera permission while it shouldn't be needed.
- once I give those, I can select which application to use to send those files. 
- Strangely, if I select "Documents" I can send something and it works while it shouldn't. 
- If I select whatever else between camera, video, other filepicker like those I installed or the Pictures app and so on, I select it but then it fails silently due to the missing storage permission. In this case, no pop-up to require it.

Comment 6

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8903599 [details] [diff] [review]
Add external storage permissions for camera/microphone usage

Review of attachment 8903599 [details] [diff] [review]:
-----------------------------------------------------------------

For the picker I think we should only request external storage if we pick a file that resides on external storage. That should be possible, right?

Comment 7

[Dylan Roeh (:droeh) (he/him)]

(In reply to Clément Lefèvre from comment #5)
> (In reply to Dylan Roeh (:droeh) from comment #4)
> > Created attachment 8903599 [details] [diff] [review]
> > Add external storage permissions for camera/microphone usage
> > 
> > It seems like there might be two issues here. I can't reproduce the problem
> > solely with filepicker -- on all three sites I can successfully choose and
> > upload photos without external storage permissions (which seems weird in and
> > of itself...). However, when I try to upload directly from the camera, it
> > fails silently as reported -- this patch fixes that issue.
> > 
> > If there's still an issue stopping upload from the filepicker I think more
> > information is needed: what device is being used and what android version is
> > installed, to start with.
> 
> So, I'm using a Sony Z3 compact with base Sony image on Android 6.0.1
> 
> after further testing:
> 
> If I remove all permissions from Nightly and try to send files on
> send.firefox.com or lut.im I could notice that:
> - Before asking what to use to send a file, Nightly still requires
> microphone and camera permission while it shouldn't be needed.
> - once I give those, I can select which application to use to send those
> files. 
> - Strangely, if I select "Documents" I can send something and it works while
> it shouldn't. 
> - If I select whatever else between camera, video, other filepicker like
> those I installed or the Pictures app and so on, I select it but then it
> fails silently due to the missing storage permission. In this case, no
> pop-up to require it.

I was testing using "Documents", which explains (sort of) why it was working even without the patch for me. I've tested and confirmed that my patch seems to fix uploading from other sources (eg the google Photos app).

Comment 8

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8903599 [details] [diff] [review]
Add external storage permissions for camera/microphone usage

Review of attachment 8903599 [details] [diff] [review]:
-----------------------------------------------------------------

Eh, I guess this is better than nothing. Can you file a followup to only request external permission when we need it?

Comment 9

[Clément Lefèvre]

> I was testing using "Documents", which explains (sort of) why it was working
> even without the patch for me. I've tested and confirmed that my patch seems
> to fix uploading from other sources (eg the google Photos app).

However, do we agree upload even from Documents shouldn't be possible without storage permission?
This is rather an Android bug than a Firefox one I guess though?

And is there already a filed bug for the microphone and camera systematic request?
When I was investigating the current bug, I was told on #mobile IRC channel it was a known one. It was some times ago and is still here though.

Comment 10

[Dylan Roeh (:droeh) (he/him)]

(In reply to Clément Lefèvre from comment #9)
> > I was testing using "Documents", which explains (sort of) why it was working
> > even without the patch for me. I've tested and confirmed that my patch seems
> > to fix uploading from other sources (eg the google Photos app).
> 
> However, do we agree upload even from Documents shouldn't be possible
> without storage permission?
> This is rather an Android bug than a Firefox one I guess though?
> 
> And is there already a filed bug for the microphone and camera systematic
> request?
> When I was investigating the current bug, I was told on #mobile IRC channel
> it was a known one. It was some times ago and is still here though.

I'm not sure if there's an existing bug; I'll file a followup to this as snorp suggested to try to improve permissions handling so that we don't request unnecessary permissions (for external storage or camera/microphone).

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e41059ef962d
Add external storage to permissions required in FilePicker.getPermissionsForMimeType. r=snorp

Comment 12

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

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

Comment 13

[Ryan VanderMeulen [:RyanVM]]

Please request Beta approval on this when you get a chance.

Comment 14

[Dylan Roeh (:droeh) (he/him)]

Comment on attachment 8903599 [details] [diff] [review]
Add external storage permissions for camera/microphone usage

Approval Request Comment
[Feature/Bug causing the regression]:
[User impact if declined]: Some file uploads will invisibly fail
[Is this code covered by automated tests?]: No
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]: 
[List of other uplifts needed for the feature/fix]:
[Is the change risky?]: No
[Why is the change risky/not risky?]: We simply add the correct permissions necessary
[String changes made/needed]: None

Comment 15

[Gerry Chang [:gchang]]

Hi Florin, could you help find someone to verify if this issue was fixed as expected on the latest Nightly build? Thanks!

Comment 16

[Gerry Chang [:gchang]]

Comment on attachment 8903599 [details] [diff] [review]
Add external storage permissions for camera/microphone usage

Fix a storage permissions popup issue. Beta56+.

Comment 17

[[SV Manager] Florin Mezei, QA (:FlorinMezei)]

Moving to Ioana fr verification on Android.

Comment 18

[Clément Lefèvre]

I verified it, it now asks for the storage permission.
I think now this bug is fine and verified (changing state as is), I was waiting for the beta discussions.

Follow-up is now in bug https://bugzilla.mozilla.org/show_bug.cgi?id=1395996

Comment 19

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/3b2becf0295b

Comment 20

[Ioana Chiorean]

As per comment #18 and verification on 57.0b13