Closed Bug 1610939 Opened 4 years ago Closed 4 years ago

The camera can't be accessed by the Fenix app on a fresh install

Categories

(GeckoView :: General, defect, P1)

74 Branch
Unspecified
All
defect

Tracking

(firefox75 fixed)

RESOLVED FIXED
mozilla75
Tracking Status
firefox75 --- fixed

People

(Reporter: j, Assigned: dminor)

Details

(Whiteboard: [geckoview:m75])

Attachments

(2 files)

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:74.0) Gecko/20100101 Firefox/74.0

Steps to reproduce:

Steps to reproduce

  1. Go to talky.io;
  2. Create a new room;
  3. Try to share the camera.

crosspost from: https://github.com/mozilla-mobile/fenix/issues/6787

Actual results:

The user can't share the camera.

Expected results:

The user can share the camera.

Rank: 10
Priority: -- → P2
Rank: 10
Priority: P2 → P1
Whiteboard: [geckoview:m75]

Hello! This is high priority for Fenix -- could I get a quick update as to status? Thank you!

Assignee: nobody → esawin
Status: UNCONFIRMED → NEW
Ever confirmed: true

This is a regression from bug 1450762 where we disable Android permission prompts for enumerateDevices.
Some sites will not attempt to getUserMedia if enumerateDevices returns no available devices for a given type.

There are multiple solutions to resolve this:

  1. wontfix
  2. Return dummy devices (e.g., by enabling privacy.resistFingerprinting or exposing this as a setting)
  3. Delegate the decision to the app by always calling into the GV permission delegate, but extend the API to include the reason/trigger

snorp, what's your opinion on this?

Flags: needinfo?(snorp)

I'm leaning towards 1) because I'm pretty sure we're doing what Chrome does now. I guess if we're not then we should for webcompat reasons. Dan, what do you think?

Flags: needinfo?(snorp) → needinfo?(dminor)

(In reply to James Willcox (:snorp) (jwillcox@mozilla.com) (he/him) from comment #3)

I'm leaning towards 1) because I'm pretty sure we're doing what Chrome does now. I guess if we're not then we should for webcompat reasons. Dan, what do you think?

Chrome returns something without prompting, those sites don't break with Chrome.

This was a site issue, I tested the STR and talky.io is working for me now with Firefox Preview and a geckoview_example build from last week. I'll defer to Jan-Ivar as this is his area of expertise.

Flags: needinfo?(dminor) → needinfo?(jib)

(In reply to James Willcox (:snorp) (jwillcox@mozilla.com) (he/him) from comment #3)

I'm leaning towards 1) because I'm pretty sure we're doing what Chrome does now.

No, Chrome for Android is able to enumerate devices without permission, as seen in this screenshot of https://fiddle.jshell.net/jib1/LbtxeLvw/show (go to Applications / Chrome and revoke Camera and Microphone permissions first).

I think that should be our goal, but neither Fennec nor Fenix seem able to do it. Do we know how Chrome does it?

If we're not able to, I think the web compatible thing would be to fake it with dummy devices until we either have real devices or know we can't get them.

Flags: needinfo?(jib) → needinfo?(dminor)

(Note the 📹 symbol in Android's notification bar in the screenshot in comment 6 is from Fenix, even though no page is accessing it, a different bug).

Beware of bug 1616729 when reproing with jsfiddle.

This should have been fixed by the patches on Bug 1578073, but I snatched failure from the jaws of success by leaving the old permissions check in the enumeration code at [1]. I'll prepare a patch for this.

[1] https://searchfox.org/mozilla-central/rev/fca0be7e2cf2f922c9b927423ce28e8a04b3fd90/dom/media/systemservices/android_video_capture/java/src/org/webrtc/videoengine/VideoCaptureDeviceInfoAndroid.java#40

Assignee: esawin → dminor
Flags: needinfo?(dminor)

This removes an unnecessary permissions check that is left over from the
previous version of this code. If Camera2Enumerator is supported by the device,
it can enumerate devices without the camera permission. If Camera1Enumerator is
used, it will attempt to open the camera and fail, returning a default value.
My local device uses Camera2Enumerator and the emulator uses Camera1Enumerator.
I tested both code paths manually and enumeration is working as expected.

I think enough devices support the newer camera API that it is not worth the
trouble of reporting a dummy camera for devices that hit the Camera1Enumerator
code path without having previously granted the camera permission.

(In reply to Dan Minor [:dminor] from comment #10)

Created attachment 9127851 [details]
Bug 1610939 - Remove unnecessary camera permissions check from VideoCaptureDeviceInfoAndroid; r=esawin!

This removes an unnecessary permissions check that is left over from the
previous version of this code. If Camera2Enumerator is supported by the device,
it can enumerate devices without the camera permission. If Camera1Enumerator is
used, it will attempt to open the camera and fail, returning a default value.
My local device uses Camera2Enumerator and the emulator uses Camera1Enumerator.
I tested both code paths manually and enumeration is working as expected.

I think enough devices support the newer camera API that it is not worth the
trouble of reporting a dummy camera for devices that hit the Camera1Enumerator
code path without having previously granted the camera permission.

GV supports API level 16+, this is only a valid solution for 21+.
This is a step forward and I think we should take this change into GV 75 now, but consider reporting dummy devices for API level 16-20.

(In reply to Eugen Sawin [:esawin] from comment #11)

(In reply to Dan Minor [:dminor] from comment #10)

Created attachment 9127851 [details]
Bug 1610939 - Remove unnecessary camera permissions check from VideoCaptureDeviceInfoAndroid; r=esawin!

This removes an unnecessary permissions check that is left over from the
previous version of this code. If Camera2Enumerator is supported by the device,
it can enumerate devices without the camera permission. If Camera1Enumerator is
used, it will attempt to open the camera and fail, returning a default value.
My local device uses Camera2Enumerator and the emulator uses Camera1Enumerator.
I tested both code paths manually and enumeration is working as expected.

I think enough devices support the newer camera API that it is not worth the
trouble of reporting a dummy camera for devices that hit the Camera1Enumerator
code path without having previously granted the camera permission.

GV supports API level 16+, this is only a valid solution for 21+.
This is a step forward and I think we should take this change into GV 75 now, but consider reporting dummy devices for API level 16-20.

Fair enough, I've filed Bug 1616904 to follow up on this.

Pushed by dminor@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b11db4268475
Remove unnecessary camera permissions check from VideoCaptureDeviceInfoAndroid; r=esawin
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla75
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: