Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Closes #967 Widevine EME playback UI support and URL Bar standardization #3082

Merged
merged 2 commits into from Apr 9, 2020
Merged

Closes #967 Widevine EME playback UI support and URL Bar standardization #3082

merged 2 commits into from Apr 9, 2020

Conversation

keianhzo
Copy link
Collaborator

@keianhzo keianhzo commented Apr 1, 2020 •
edited

Closes #967 Closes #3075 This PR adds support for Widevine EME playback UI support. Also unifies the toolbar permission button bubbles using a switch instead of a button.

Testing site: https://www.ezdrm.com/html/drm-demos.asp

@keianhzo keianhzo changed the title V10/drm Closes #967 Widevine EME playback UI support Apr 1, 2020
@keianhzo keianhzo self-assigned this Apr 1, 2020
@keianhzo keianhzo added this to the #10 features milestone Apr 1, 2020
@keianhzo keianhzo changed the title Closes #967 Widevine EME playback UI support Closes #967 Widevine EME playback UI support and URL Bar standardization Apr 1, 2020
bluemarvin
bluemarvin suggested changes Apr 2, 2020
View reviewed changes
Copy link
Contributor

@bluemarvin bluemarvin left a comment

Choose a reason for hiding this comment

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

I never see a user prompt to enable the first time a DRM video is attempted to be played.

app/src/common/shared/org/mozilla/vrbrowser/browser/PermissionDelegate.java Outdated Show resolved Hide resolved
@keianhzo
Copy link
Collaborator Author

keianhzo commented Apr 3, 2020

Icons updated

@keianhzo keianhzo marked this pull request as ready for review April 7, 2020 16:42
@keianhzo
Copy link
Collaborator Author

keianhzo commented Apr 7, 2020

@bluemarvin @MortimerGoro this should be ready for review.

@keianhzo keianhzo requested a review from bluemarvin April 7, 2020 16:43
@bluemarvin
Copy link
Contributor

@keianhzo I think the popup should match the settings and say on/off?
org mozilla vrbrowser-20200407-165408

@keianhzo
Copy link
Collaborator Author

keianhzo commented Apr 8, 2020 •
edited

@bluemarvin I've unified the settings to "Advanced" and all the bubbles to "on/off" I thinkit makes it more clear.

Also it seems that there are DRM issues in Pico devices:

2020-04-08 12:26:51.292 4238-4277/org.mozilla.vrbrowser.dev:tab0 I/Web Content: MediaKeySystemAccess::GetKeySystemStatus(com.widevine.alpha) result=cdm-not-supported msg='KeySystem or Minimum API level not met for Widevine EME'
2020-04-08 12:26:51.292 4238-4277/org.mozilla.vrbrowser.dev:tab0 I/Web Content: navigator.requestMediaKeySystemAccess promise rejected 0x80530009 'KeySystem or Minimum API level not met for Widevine EME'
2020-04-08 12:26:51.293 4238-4277/org.mozilla.vrbrowser.dev:tab0 I/Web Content: navigator.requestMediaKeySystemAccess promise rejected 0x80530009 'Key system is unsupported'
2020-04-08 12:26:51.298 1139-1259/? I/DrmHal: makeDrmFactories: factory instance default is Remote
2020-04-08 12:26:51.298 4238-4277/org.mozilla.vrbrowser.dev:tab0 I/Web Content: MediaKeySystemAccess::GetKeySystemStatus(com.widevine.alpha) result=cdm-not-supported msg='KeySystem or Minimum API level not met for Widevine EME'
2020-04-08 12:26:51.298 4238-4277/org.mozilla.vrbrowser.dev:tab0 I/Web Content: navigator.requestMediaKeySystemAccess promise rejected 0x80530009 'KeySystem or Minimum API level not met for Widevine EME'
2020-04-08 12:26:51.299 4238-4277/org.mozilla.vrbrowser.dev:tab0 I/Web Content: navigator.requestMediaKeySystemAccess promise rejected 0x80530009 'Key system is unsupported'

I tried with and without e10s.

@keianhzo
Copy link
Collaborator Author

keianhzo commented Apr 8, 2020

@bluemarvin I've hidden DRM for Pico devices

@bluemarvin
Copy link
Contributor

I'm still seeing Allow for the bubble:
org mozilla vrbrowser-20200408-090218

@bluemarvin
Copy link
Contributor

bluemarvin commented Apr 8, 2020 •
edited

Additionally, the page does not need to be reloaded when accepting DRM for the first time from the dialog pop up.

bluemarvin
bluemarvin suggested changes Apr 8, 2020
View reviewed changes
Copy link
Contributor

@bluemarvin bluemarvin left a comment

Choose a reason for hiding this comment

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

The popup DRM bubble needs to say On/Off instead of Allow/Don't Allow and we should not reload the page when accepting DRM the first time.

@keianhzo
Copy link
Collaborator Author

keianhzo commented Apr 9, 2020

@bluemarvin Fixed first reload and switch texts

@bluemarvin bluemarvin merged commit ec3b681 into master Apr 9, 2020
@bluemarvin bluemarvin deleted the v10/drm branch April 9, 2020 18:21
@denschub denschub mentioned this pull request Jun 4, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bluemarvin bluemarvin bluemarvin approved these changes

@MortimerGoro MortimerGoro Awaiting requested review from MortimerGoro

Assignees

@keianhzo keianhzo

Labels
None yet
Projects
None yet
Milestone
#10 features
Development

Successfully merging this pull request may close these issues.

Update Pop-up Blocking Override UI Widevine EME playback UI support
2 participants
@keianhzo @bluemarvin