Bug 143350

Summary: REGRESSION: Caption Menus show language codes instead of display names
Product: WebKit Reporter: Brent Fulgham <bfulgham>
Component: MediaAssignee: Brent Fulgham <bfulgham>
Status: RESOLVED FIXED    
Severity: Critical CC: bfulgham, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch
none
Patch jer.noble: review+

Brent Fulgham
Reported 2015-04-02 16:11:14 PDT
The menu of audio tracks presented by WebKit is showing language codes (e.g., "de-DE", or "fr-CA"), instead of display names (e.g, "German (Germany)" or "French (Canada)". This seems to have been introduced in <http://trac.webkit.org/r174402>.
Attachments
Patch (9.86 KB, patch)
2015-04-02 16:21 PDT, Brent Fulgham 		no flags
DetailsFormatted DiffDiff
Patch (6.87 KB, patch)
2015-04-02 16:45 PDT, Brent Fulgham 		jer.noble: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Brent Fulgham
Comment 1 2015-04-02 16:11:47 PDT
<rdar://problem/20094145>
Brent Fulgham
Comment 2 2015-04-02 16:21:48 PDT
Created attachment 250018 [details] Patch
Jer Noble
Comment 3 2015-04-02 16:27:29 PDT
Comment on attachment 250018 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=250018&action=review r=me, with possible nit. > Source/WebCore/page/CaptionUserPreferencesMediaAF.cpp:611 > + String label = track->label(); > + String trackLanguageIdentifier = track->language(); > + > + RetainPtr<CFLocaleRef> currentLocale = adoptCF(CFLocaleCreate(kCFAllocatorDefault, defaultLanguage().createCFString().get())); > + RetainPtr<CFStringRef> localeIdentifier = adoptCF(CFLocaleCreateCanonicalLocaleIdentifierFromString(kCFAllocatorDefault, trackLanguageIdentifier.createCFString().get())); > + RetainPtr<CFStringRef> languageCF = adoptCF(CFLocaleCopyDisplayNameForPropertyValue(currentLocale.get(), kCFLocaleLanguageCode, localeIdentifier.get())); > + String language = languageCF.get(); This looks like duplicated code from trackDisplayName(AudioTrack*). Should we pull this into buildStringForLocaleAndLanguage()? (And perhaps rename it to buildStringForDefaultLocaleAndLanguage()?)
Brent Fulgham
Comment 4 2015-04-02 16:30:34 PDT
Comment on attachment 250018 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=250018&action=review >> Source/WebCore/page/CaptionUserPreferencesMediaAF.cpp:611 >> + String language = languageCF.get(); > > This looks like duplicated code from trackDisplayName(AudioTrack*). Should we pull this into buildStringForLocaleAndLanguage()? (And perhaps rename it to buildStringForDefaultLocaleAndLanguage()?) Sure!
Brent Fulgham
Comment 5 2015-04-02 16:45:51 PDT
Created attachment 250020 [details] Patch
Brent Fulgham
Comment 6 2015-04-02 17:27:54 PDT
Committed r182298: <http://trac.webkit.org/changeset/182298>
Darin Adler
Comment 7 2015-04-03 13:22:44 PDT
Comment on attachment 250020 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=250020&action=review > Source/WebCore/page/CaptionUserPreferencesMediaAF.cpp:582 > +static String trackDisplayName(AudioTrack* track) Should take a reference. > Source/WebCore/page/CaptionUserPreferencesMediaAF.cpp:588 > + displayName.append(audioTrackNoLabelText()); Should just return here. No real reason to append to an empty string builder and then convert back into a string. > Source/WebCore/page/CaptionUserPreferencesMediaAF.cpp:593 > +String CaptionUserPreferencesMediaAF::displayNameForTrack(AudioTrack* track) const Should take a reference. > Source/WebCore/page/CaptionUserPreferencesMediaAF.cpp:598 > +static String trackDisplayName(TextTrack* track) Should take a reference.
Note You need to log in before you can comment on or make changes to this bug.