Bug 143350 – REGRESSION: Caption Menus show language codes instead of display names

• Status: RESOLVED
• Resolution: FIXED
• Component: Media

[Brent Fulgham]

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>.

[Brent Fulgham]

<rdar://problem/20094145>

[Brent Fulgham]

Created attachment 250018 [details]
Patch

[Jer Noble]

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 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]

Created attachment 250020 [details]
Patch

[Brent Fulgham]

Committed r182298: <http://trac.webkit.org/changeset/182298>

[Darin Adler]

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.