Closed
Bug 1262556
Opened 8 years ago
Closed 8 years ago
Land user-facing strings for Decoder Doctor work
Categories
(Core :: Audio/Video: Playback, defect)
Core
Audio/Video: Playback
Tracking
()
RESOLVED
FIXED
mozilla48
Tracking | Status | |
---|---|---|
firefox46 | --- | unaffected |
firefox47 | --- | fixed |
firefox48 | --- | fixed |
People
(Reporter: jaws, Assigned: jaws)
References
Details
Attachments
(1 file, 1 obsolete file)
1.34 KB,
patch
|
jaws
:
review+
ritu
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
Decoder Doctor (bug 1160424) will introduce a couple notification bars that will have strings related to potential issues with missing codecs and poor audio video playback performance. We may want to uplift these strings to 47, but in the meantime we will want to make sure that they are on the 48 train.
Assignee | ||
Comment 1•8 years ago
|
||
I am planning to land these as regular strings under /browser/locales, as opposed to the way that Pocket landed them in https://bugzilla.mozilla.org/show_bug.cgi?id=1162283. This is because even if we uplift strings, we will want translations for all locales.
Assignee | ||
Comment 2•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/44659/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/44659/
Attachment #8738711 -
Flags: review?(dolske)
Assignee | ||
Comment 3•8 years ago
|
||
The strings in the attached patch are a bit different than the one in the Google Doc that I received. I got these updated strings from Matej while working on the patch and I've updated the Google Doc to have these new strings.
Comment 4•8 years ago
|
||
Comment on attachment 8738711 [details] MozReview Request: Bug 1262556 - Land user-facing strings for Decoder Doctor work. r?dolske https://reviewboard.mozilla.org/r/44659/#review41471 I also spent a minute thinking about the use of "you may need to" -- would be nice if we could be less wishy-washy about what the user should do. But it's not a big deal, and probably makes sense for the inevitable(?) edge cases where a page manages to trigger these notifications, but then falls back to something that works for the user anyway. ::: browser/locales/en-US/chrome/browser/decoder.properties:7 (Diff revision 1) > +# License, v. 2.0. If a copy of the MPL was not distributed with this > +# file, You can obtain one at http://mozilla.org/MPL/2.0/. > + > +decoder.noCodecs.button = Learn how > +decoder.noCodecs1.message = To play video, you may need to update your system preferences. > +decoder.noCodecs2.message = To play video, you may need to install Microsoft’s Media Feature Pack. Nit: Probably shouldn't use "1" and "2" here, since if we change these strings in the future that could get confusing (i.e., adding a number to rev the property name for L10N) But... Do we actually need both strings? I think the draft UX doc just had these as two separate suggestions for the same thing. The latter is more descriptive, so would be my preference. ::: browser/locales/jar.mn:29 (Diff revision 1) > locale/browser/aboutSyncTabs.dtd (%chrome/browser/aboutSyncTabs.dtd) > locale/browser/browser.dtd (%chrome/browser/browser.dtd) > locale/browser/baseMenuOverlay.dtd (%chrome/browser/baseMenuOverlay.dtd) > locale/browser/browser.properties (%chrome/browser/browser.properties) > locale/browser/customizableui/customizableWidgets.properties (%chrome/browser/customizableui/customizableWidgets.properties) > + locale/browser/decoder.properties (%chrome/browser/decoder.properties) A separate file is fine, but I suspect it's not actually needed for an early string uplift. Might roll it into browser.properties if L10N is cool with that.
Attachment #8738711 -
Flags: review?(dolske) → review+
Comment 5•8 years ago
|
||
My sense is that the strings from the gdoc are were not final or a finely-crafted multi-party compromise, but NI cpeterson just to make sure there's no technical objection to the revisions.
Flags: needinfo?(cpeterson)
Assignee | ||
Comment 6•8 years ago
|
||
Attachment #8738711 -
Attachment is obsolete: true
Attachment #8739179 -
Flags: review+
Assignee | ||
Updated•8 years ago
|
Keywords: checkin-needed
Comment 7•8 years ago
|
||
bugherder landing |
https://hg.mozilla.org/integration/mozilla-inbound/rev/0df02c629e77
Keywords: checkin-needed
Comment 8•8 years ago
|
||
(In reply to Justin Dolske [:Dolske] from comment #5) > My sense is that the strings from the gdoc are were not final or a > finely-crafted multi-party compromise, but NI cpeterson just to make sure > there's no technical objection to the revisions. LGTM. We haven't finalized the strings, but I don't see any technical reason to not land Jared's suggested strings in the meantime. We might want to tweak the wording of the strings, but no significant changes that would break the meaning of the translated strings.
Flags: needinfo?(cpeterson)
Comment 9•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/0df02c629e77
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox48:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Comment 10•8 years ago
|
||
Can someone clarify why we are pre-landing strings weeks before the end of cycle, but they might not even be finalized?
Assignee | ||
Comment 11•8 years ago
|
||
(In reply to Francesco Lodolo [:flod] from comment #10) > Can someone clarify why we are pre-landing strings weeks before the end of > cycle, but they might not even be finalized? The strings should be final, as per comment #3. I don't expect any changes to come forward. These strings were landed pre-emptively because there is a high chance that we may want to uplift them to Aurora and as such we would want to give localizers who want to translate them as much time as possible. We specifically will want to try to get localizations for Korean and European languages, places where Windows 10 N and KN are sold. An email was supposed to go out to dev-l10n describing these but looking at the archives it appears that hasn't been sent yet. Marking needinfo for Chris who should be sending out those emails. I will give a little background here: This bug is part of a project called Decoder Doctor that will try to help users who have video decoding issues on their local system. That project is targeting Firefox 47, if possible, and we want to be as proactive as we can with regards to getting strings translated. I can't speak any more to the reasoning behind "why 47".
Flags: needinfo?(cpeterson)
Comment 12•8 years ago
|
||
I wanted to get final sign-off from Matej before I emailed the dev-l10n list, but Jared says he had already spoken with Matej on IRC. So I could have sent the email sooner. Sorry for the mix up.
Flags: needinfo?(cpeterson)
Updated•8 years ago
|
Status: RESOLVED → REOPENED
status-firefox46:
--- → unaffected
status-firefox47:
--- → affected
Resolution: FIXED → ---
Updated•8 years ago
|
Status: REOPENED → RESOLVED
Closed: 8 years ago → 8 years ago
Resolution: --- → FIXED
Comment 13•8 years ago
|
||
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #11) > The strings should be final, as per comment #3. But then there was comment #6. Anyhow, that part seems to be explained. > We specifically will want to try to get localizations for Korean and > European languages, places where Windows 10 N and KN are sold. An email was > supposed to go out to dev-l10n describing these but looking at the archives > it appears that hasn't been sent yet. Marking needinfo for Chris who should > be sending out those emails. What are you going to say in this email? Because strings are currently in mozilla-central, so they're invisible to most of our localization teams (they work against aurora). And if you uplift them when 47 is in beta, it's even worse: they'll see them as missing, but tools won't let them localize these strings since they're bound to aurora. It would be helpful to have someone from l10n-drivers in the loop for this kind of situations (I assume it didn't happen in this case, since I've only seen the landing in m-c).
Comment 14•8 years ago
|
||
(In reply to Francesco Lodolo [:flod] from comment #13) > What are you going to say in this email? Because strings are currently in > mozilla-central, so they're invisible to most of our localization teams > (they work against aurora). And if you uplift them when 47 is in beta, it's > even worse: they'll see them as missing, but tools won't let them localize > these strings since they're bound to aurora. My email to dev-l10n provided some backstory to the strings and gave a heads-up that they're coming to Aurora 47 soon. We can uplift the strings to Aurora ASAP, unless you recommend a different course. 47 will move from Aurora to Beta on April 25. > It would be helpful to have someone from l10n-drivers in the loop for this > kind of situations (I assume it didn't happen in this case, since I've only > seen the landing in m-c). Thanks. I didn't know about l10n-drivers. My email to dev-l10n finally got through (after some email problems on my side). I will follow up with l10n-drivers.
Comment 15•8 years ago
|
||
Comment on attachment 8739179 [details] [diff] [review] Patch for check-in Approval Request Comment [Feature/regressing bug #]: bug 1180108: the "Decoder Doctor" is a new Firefox feature to notify users when they can't play an HTML5 video because they are missing a codec. [User impact if declined]: Unlike Chrome or Edge, Firefox does not ship H.264 or AAC codecs. We instead rely on the operating system's codecs. However, some Windows versions do not include Microsoft's codecs, which are available as a free download, if you know where to look. About 15% of Vista users and 1% of Windows 7–10 users are missing these codecs. These are typically the Windows "N" and "KN" editions for European countries and Korea, respectively. Support for Google's Widevine CDM will land in a few days: https://blog.mozilla.org/futurereleases/2016/04/08/mozilla-to-test-widevine-cdm-in-firefox-nightly. Unlike Adobe's Primetime CDM, Widevine does not included an AAC codec. Without the "Decoder Doctor", those 15% of Vista users and 1% of Windows 7–10 users won't be able to play Widevine content's audio. [Describe test coverage new/current, TreeHerder]: The strings landed on mozilla-central, but the code to use the strings has not landed yet. [Risks and why]: There is no stability risk to uplifting the strings, but there is a low-probability chance the code to use the strings will change. [String/UUID change made/needed]: YES!
Attachment #8739179 -
Flags: approval-mozilla-aurora?
Comment 16•8 years ago
|
||
Wait, uplift? I didn't realize that. What of https://bugzilla.mozilla.org/showdependencytree.cgi?id=1180108&hide_resolved=1 needs to uplifted to 47? I'm having concerns to get great quality here for 60-70% of our users at this point.
Comment 17•8 years ago
|
||
(In reply to Chris Peterson [:cpeterson] from comment #15) > [String/UUID change made/needed]: YES! Just for clarification, there are some new strings, but no UUID changes. (In reply to Axel Hecht [:Pike] from comment #16) > What of > https://bugzilla.mozilla.org/showdependencytree. > cgi?id=1180108&hide_resolved=1 needs to uplifted to 47? Not all of those bugs will need to be fixed. Bug 848994 and bug 1160424 are the only ones necessary to address the Widevine codec problems for Windows N/KN users.
Comment on attachment 8739179 [details] [diff] [review] Patch for check-in This is "needed" for the new CDM work, just strings so far, Aurora47+
Attachment #8739179 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 19•8 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-aurora/rev/9e79d3cbd186
Blocks: 1264807
You need to log in
before you can comment on or make changes to this bug.
Description
•