Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Add content setting for media device access #14970

Merged
merged 1 commit into from
Aug 8, 2018

Conversation

diracdeltas
Copy link
Member

@diracdeltas diracdeltas commented Aug 8, 2018

fix #14889

Test plan:

  1. go to https://www.webrtc-experiment.com/DetectRTC/
  2. confirm that (1) the results are the same as what chrome shows on
    that site and (2) you see a permission prompt for media access
  3. click 'remember this decision' and 'allow' on the permission prompt
    and reload the page.
  4. confirm that in the "System has Speakers?", "System has Microphone?", "System has Webcam" section, it now shows descriptions of all your devices.

Submitter Checklist:

  • Submitted a ticket for my issue if one did not already exist.
  • Used Github auto-closing keywords in the commit message.
  • Added/updated tests for this change (for new code or code which already has tests).
  • Ran git rebase -i to squash commits (if needed).
  • Tagged reviewers and labelled the pull request as needed.
  • Request a security/privacy review as needed. (Ask a Brave employee to help if you cannot access this document.)

Test Plan:

Reviewer Checklist:

  • Request a security/privacy review as needed if one was not already requested.

Tests

  • Adequate test coverage exists to prevent regressions
  • Tests should be independent and work correctly when run individually or as a suite ref
  • New files have MPL2 license header

@diracdeltas diracdeltas requested a review from darkdh August 8, 2018 00:53
@diracdeltas diracdeltas added this to the 0.23.x Hotfix 2 milestone Aug 8, 2018
@diracdeltas diracdeltas self-assigned this Aug 8, 2018

if (chrome.contentSettings.mediaPermission == 'block') {
// Needed for https://github.com/brave/browser-laptop/issues/14889
// Note this is not necessary in non-Electron-based codebases since Chromium
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we also add chromium doesn't have mediaPermission in content setting? It is actually microphone && camera in chromium

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated

@darkdh
Copy link
Member

darkdh commented Aug 8, 2018

also need to check System has Webcam? section in test case

fix #14889

Test plan:
1. go to https://www.webrtc-experiment.com/DetectRTC/
2. confirm that (1) the results are the same as what chrome shows on
   that site and (2) you see a permission prompt for media access
3. click 'remember this decision' and 'allow' on the permission prompt
   and reload the page.
4. confirm that in the "System has Speakers?" and "System has Microphone?" section, it now shows descriptions of all your devices.
@diracdeltas diracdeltas force-pushed the fix/media-device-access branch from dc9e04d to 4a79331 Compare August 8, 2018 01:26
@diracdeltas
Copy link
Member Author

@darkdh addressed review comments and travis passes now

@bsclifton bsclifton merged commit 436e84f into master Aug 8, 2018
@bsclifton bsclifton deleted the fix/media-device-access branch August 8, 2018 06:56
bsclifton added a commit that referenced this pull request Aug 8, 2018
Add content setting for media device access
bsclifton added a commit that referenced this pull request Aug 8, 2018
Add content setting for media device access
bsclifton added a commit that referenced this pull request Aug 8, 2018
Add content setting for media device access
@bsclifton
Copy link
Member

master 436e84f
0.24.x f193d4c
0.23.x 7aa0c2c
0.23.x-hotfix2 45a5fa2

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

navigator.mediaDevices.enumerateDevices() returns too much info
3 participants