Closed
Bug 1360863
Opened 5 years ago
Closed 5 years ago
EME prompt is not shown
Categories
(Firefox :: General, defect)
Firefox
General
Tracking
()
VERIFIED
FIXED
Firefox 55
People
(Reporter: ajones, Assigned: cpearce)
References
Details
(Whiteboard: [bugday-20170510])
Attachments
(3 files, 1 obsolete file)
59 bytes,
text/x-review-board-request
|
dao
:
review+
|
Details |
2.37 KB,
patch
|
jaws
:
review+
gchang
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
87.32 KB,
image/png
|
Details |
Steps to reproduce: * Disable EME * Attempt to play EME content on Netflix or YouTUbe Expected results: Drop down shown to enable DRM Actual results: No drop down. Unhelpful error message shown on Netflix/YouTube
Reporter | ||
Updated•5 years ago
|
Flags: needinfo?(cpearce)
Assignee | ||
Comment 1•5 years ago
|
||
A simpler test case is: https://shaka-player-demo.appspot.com/demo/#asset=//yt-dash-mse-test.commondatastorage.googleapis.com/media/oops_cenc-20121114-signedlicenseurl-manifest.mpd;lang=en-US;play This URL should play if "Play DRM Content" is checked, and show the "enable DRM" notification box if not. I see this error logged in console: *** Failed to format string emeNotifications.drmContentDisabled.message in bundle: chrome://browser/locale/browser.properties JavaScript error: XStringBundle, line 37: NS_ERROR_FAILURE: Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIStringBundle.formatStringFromName] I bet something change in the front end.
Flags: needinfo?(cpearce)
Assignee | ||
Comment 2•5 years ago
|
||
This was regressed by https://hg.mozilla.org/integration/mozilla-inbound/rev/e8a1222fd9f4 in Bug 1330001. So it shipped in 53. :(
Assignee | ||
Updated•5 years ago
|
Assignee | ||
Comment 3•5 years ago
|
||
This bug causes us to not show the "Enable DRM playback" notification box when users visit Netflix.com or any other site that tries to use DRM playback while DRM playback is disabled. We ship with DRM playback disabled by default on Linux (only). So this bug means that Linux users won't be prompted to enable DRM playback, and so sites trying to use DRM will not work and there will be no obvious way to make them work!
status-firefox53:
--- → affected
status-firefox54:
--- → affected
status-firefox55:
--- → affected
tracking-firefox53:
--- → ?
tracking-firefox54:
--- → ?
tracking-firefox55:
--- → ?
Comment hidden (mozreview-request) |
Assignee | ||
Updated•5 years ago
|
Assignee: nobody → cpearce
Once this lands, let's verify the fix and uplift to at least 54.
Comment 7•5 years ago
|
||
mozreview-review |
Comment on attachment 8863290 [details] Bug 1360863 - Unremove emeNotifications.drmContentDisabled.message. https://reviewboard.mozilla.org/r/135096/#review138512
Attachment #8863290 -
Flags: review?(dao+bmo) → review+
Pushed by cpearce@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/2ec98c2a4824 Unremove emeNotifications.drmContentDisabled.message. r=dao
Comment 9•5 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/2ec98c2a4824
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
Comment 10•5 years ago
|
||
(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #5) > Once this lands, let's verify the fix and uplift to at least 54. The patch for uplift should have the string hard-coded. The error it's already in release, which means we lost that string all over the place.
Comment 11•5 years ago
|
||
I have reproduced this bug with Nightly 55.0a1 (2017-04-29) on Windows 8.1 , 64 Bit ! This bug's fix is Verified with latest Nightly ! Build ID 20170503030212 User Agent Mozilla/5.0 (Windows NT 6.3; WOW64; rv:55.0) Gecko/20100101 Firefox/55.0 [bugday-20170503]
Updated•5 years ago
|
status-firefox-esr52:
--- → unaffected
Comment 12•5 years ago
|
||
Hi :cpearce, Could you please nominate patch for uplift to Bet54?
Flags: needinfo?(cpearce)
Assignee | ||
Comment 13•5 years ago
|
||
Assignee | ||
Comment 14•5 years ago
|
||
Comment on attachment 8865382 [details] [diff] [review] Beta rebase with string hardcoded (In reply to Francesco Lodolo [:flod] from comment #10) > (In reply to Liz Henry (:lizzard) (needinfo? me) from comment #5) > > Once this lands, let's verify the fix and uplift to at least 54. > > The patch for uplift should have the string hard-coded. The error it's > already in release, which means we lost that string all over the place. Like this?
Flags: needinfo?(francesco.lodolo)
Comment 15•5 years ago
|
||
I don't think I've ever seen a nested ternary operator, but yes, it should do the right: read the hard-coded string if the ID is emeNotifications.drmContentDisabled.message, otherwise fall back to the "proper" code path.
Flags: needinfo?(francesco.lodolo)
Assignee | ||
Comment 16•5 years ago
|
||
Comment on attachment 8865382 [details] [diff] [review] Beta rebase with string hardcoded I have tested this code and it works as expected, but would be good to get a once over from a front end reviewer before requesting uplift to beta.
Flags: needinfo?(cpearce)
Attachment #8865382 -
Flags: review?(jaws)
Comment 17•5 years ago
|
||
Comment on attachment 8865382 [details] [diff] [review] Beta rebase with string hardcoded Review of attachment 8865382 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/base/content/browser-media.js @@ +103,5 @@ > > let msgPrefix = "emeNotifications." + notificationId + "."; > let msgId = msgPrefix + "message"; > > + let message = (notificationId == "drmContentDisabled") ? We should try to load the string from the .properties file first, and if that fails then fall back to the hard coded string. As this patch is written, locales won't be able to fix it if they want to. Can we wrap the getFormattedString call in a try/catch and then if the msgId is "emeNotifications.drmContentDisabled.message" and an exception is thrown we will use the hard-coded string?
Attachment #8865382 -
Flags: review?(jaws) → review-
Comment 18•5 years ago
|
||
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #17) > We should try to load the string from the .properties file first, and if > that fails then fall back to the hard coded string. As this patch is > written, locales won't be able to fix it if they want to. This patch is only for Beta (maybe release), we don't have the string available there, so there's not much of a point in trying to load it.
Assignee | ||
Comment 19•5 years ago
|
||
Attachment #8866018 -
Flags: review?(jaws)
Assignee | ||
Updated•5 years ago
|
Attachment #8865382 -
Attachment is obsolete: true
Comment 20•5 years ago
|
||
Comment on attachment 8866018 [details] [diff] [review] Beta rebase with string hardcoded Review of attachment 8866018 [details] [diff] [review]: ----------------------------------------------------------------- Thanks!
Attachment #8866018 -
Flags: review?(jaws) → review+
Assignee | ||
Comment 21•5 years ago
|
||
Comment on attachment 8866018 [details] [diff] [review] Beta rebase with string hardcoded Approval Request Comment [Feature/Bug causing the regression]: DRM protected video playback (i.e. Netflix). This is a regression from Bug 1330001 which accidentally removed a string. [User impact if declined]: Users who have "Play DRM Content" unchecked in Firefox's preferences (which is by default all Linux users) will see sites that use DRM playback like Netflix fail with no indication from our UI as to why they had failed. [Is this code covered by automated tests?]: No. [Has the fix been verified in Nightly?]: No. I have verified with a local build. The fix to Nightly re-added the string. But because we can't take string changes to beta (right?), this patch hard codes the string in English. So it's a completely different patch to what landed on Nightly. [Needs manual test from QE? If yes, steps to reproduce]: Yes please. Disable DRM playback in about:preferences, load https://shaka-player-demo.appspot.com/ and you should see a notification box drop down "You must enable DRM to play some audio or video on this page." [List of other uplifts needed for the feature/fix]: None. [Is the change risky?]: No [Why is the change risky/not risky?]: It's simple. [String changes made/needed]: Yes, this patch hard codes a string was removed accidentally with an English string. This patch also checks to see whether the string is there first, so if our translators could re-add the fix we'd be able to gracefully start using the localized string.
Attachment #8866018 -
Flags: approval-mozilla-beta?
Assignee | ||
Comment 22•5 years ago
|
||
We should really get a test for this... maybe a browser chrome mochitest? I thought we had one already for this feature...
Flags: in-testsuite?
Comment 23•5 years ago
|
||
Hi Brindusa, could you help find someone to verify if this issue was fixed as expected on a latest Nightly build? Thanks!
Flags: qe-verify+
Flags: needinfo?(brindusa.tot)
Comment 24•5 years ago
|
||
¡Hola Gerry! FWIW I do get the prompt. Nombre Firefox Versión 55.0a1 Id. de compilación 20170510183715 ¡Gracias! Alex
Status: RESOLVED → VERIFIED
Comment 25•5 years ago
|
||
Updated•5 years ago
|
Whiteboard: [bugday-20170510]
Comment 26•5 years ago
|
||
Comment on attachment 8866018 [details] [diff] [review] Beta rebase with string hardcoded Fix an EME prompt issue and was verified. Beta54+. Should be in 54 beta 7.
Attachment #8866018 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Updated•5 years ago
|
Flags: needinfo?(brindusa.tot)
Comment 27•5 years ago
|
||
Sorry, yesterday was a full day. Verified as fixed on latest Nightly 55.0a1, Build ID 20170510030301 on Windows 10 x64, Ubuntu 16.04 x64 and Mac OS X 10.12.
Comment 28•5 years ago
|
||
bugherderuplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/ea3a47d36b6a
I have managed to reproduce this issue on Firefox 53 using STR from comment 0. This is verified fixed on 54 beta 9 across platforms: Windows 10 x64, Ubuntu 16.04 x64 LTS and Mac OS X 10.10.5.
Flags: qe-verify+
Assignee | ||
Comment 30•5 years ago
|
||
I added a test for this feature in bug 1366167.
Flags: in-testsuite? → in-testsuite+
You need to log in
before you can comment on or make changes to this bug.
Description
•