Closed Bug 1390261 Opened 7 years ago Closed 7 years ago

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

Categories

(Firefox for Android Graveyard :: General, defect)

defect
Not set
normal

Tracking

(fennec+, firefox55 wontfix, firefox56 verified, firefox57 verified)

VERIFIED FIXED
Firefox 57
Tracking Status
fennec + ---
firefox55 --- wontfix
firefox56 --- verified
firefox57 --- verified

People

(Reporter: clement.lefevre, Assigned: droeh)

References

Details

(Keywords: nightly-community, regression, regressionwindow-wanted, Whiteboard: [mozfr-community])

Attachments

(1 file)

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
Whiteboard: [mozfr-community]
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.
Assignee: nobody → droeh
tracking-fennec: --- → ?
Flags: needinfo?(droeh)
These seems to <input type="file" accept="....> so may be caused by my bug 1337692
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?
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.
Flags: needinfo?(droeh) → needinfo?(clement.lefevre)
Attachment #8903599 - Flags: review?(snorp)
(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.
Flags: needinfo?(clement.lefevre)
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?
Attachment #8903599 - Flags: review?(snorp) → review-
(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 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?
Attachment #8903599 - Flags: review- → review+
> 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.
(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).
Pushed by droeh@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e41059ef962d
Add external storage to permissions required in FilePicker.getPermissionsForMimeType. r=snorp
Blocks: 1395996
https://hg.mozilla.org/mozilla-central/rev/e41059ef962d
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
Please request Beta approval on this when you get a chance.
Flags: needinfo?(droeh)
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
Flags: needinfo?(droeh)
Attachment #8903599 - Flags: approval-mozilla-beta?
Hi Florin, could you help find someone to verify if this issue was fixed as expected on the latest Nightly build? Thanks!
Flags: qe-verify+
Flags: needinfo?(florin.mezei)
Comment on attachment 8903599 [details] [diff] [review]
Add external storage permissions for camera/microphone usage

Fix a storage permissions popup issue. Beta56+.
Attachment #8903599 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Moving to Ioana fr verification on Android.
Flags: needinfo?(florin.mezei) → needinfo?(ioana.chiorean)
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
Status: RESOLVED → VERIFIED
As per comment #18 and verification on 57.0b13
Flags: needinfo?(ioana.chiorean)
Flags: qe-verify+
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.