Bug 120956 – Regression: media controls and status messages are no longer localized

• Component: New Bugs
• Status: RESOLVED FIXED

[Jer Noble]

[Mac] Localize the JavaScript controls

[James Craig]

related to bug 121990

[Radar WebKit Bug Importer]

<rdar://problem/15365231>

[James Craig]

Blocking against bug 123749 because I've got more strings coming in that one.

[Brent Fulgham]

Created attachment 229151 [details]
Patch

[Jer Noble]

Comment on attachment 229151 [details]
Patch

Nice! r=me.

[Jon Lee]

Comment on attachment 229151 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=229151&action=review

> Source/WebCore/English.lproj/mediaControlsLocalizedStringsiOS.js:4
> +        '##AIRPLAY_DEVICE_NAME##': 'This video is playing on "##DEVICE_NAME##".',

These should be the nicer quotes!

[Brent Fulgham]

Committed r167145: <http://trac.webkit.org/changeset/167145>

[Brent Fulgham]

(In reply to comment #6)
> (From update of attachment 229151 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=229151&action=review
> 
> > Source/WebCore/English.lproj/mediaControlsLocalizedStringsiOS.js:4
> > +        '##AIRPLAY_DEVICE_NAME##': 'This video is playing on "##DEVICE_NAME##".',
> 
> These should be the nicer quotes!

I don't understand what you mean here. Is there a UNICODE character for start/end quote we should be using?

[Jon Lee]

(In reply to comment #8)
> (In reply to comment #6)
> > (From update of attachment 229151 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=229151&action=review
> > 
> > > Source/WebCore/English.lproj/mediaControlsLocalizedStringsiOS.js:4
> > > +        '##AIRPLAY_DEVICE_NAME##': 'This video is playing on "##DEVICE_NAME##".',
> > 
> > These should be the nicer quotes!
> 
> I don't understand what you mean here. Is there a UNICODE character for start/end quote we should be using?

“##DEVICE_NAME##” rather than "##DEVICE_NAME##"

[Brent Fulgham]

Curl quotes landed in follow-up:
r167147 <http://trac.webkit.org/changeset/167147>

[Xabier Rodríguez Calvar]

Comment on attachment 229151 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=229151&action=review

> Source/WebCore/rendering/RenderThemeMac.mm:245
> -    if (m_mediaControlsScript.isEmpty())
> -        m_mediaControlsScript = [NSString stringWithContentsOfFile:[[NSBundle bundleForClass:[WebCoreRenderThemeBundle class]] pathForResource:@"mediaControlsApple" ofType:@"js"] encoding:NSUTF8StringEncoding error:nil];
> +    if (m_mediaControlsScript.isEmpty()) {
> +        StringBuilder scriptBuilder;
> +        scriptBuilder.append([NSString stringWithContentsOfFile:[[NSBundle bundleForClass:[WebCoreRenderThemeBundle class]] pathForResource:@"mediaControlsLocalizedStrings" ofType:@"js"] encoding:NSUTF8StringEncoding error:nil]);
> +        scriptBuilder.append([NSString stringWithContentsOfFile:[[NSBundle bundleForClass:[WebCoreRenderThemeBundle class]] pathForResource:@"mediaControlsApple" ofType:@"js"] encoding:NSUTF8StringEncoding error:nil]);
> +        m_mediaControlsScript = scriptBuilder.toString();
> +    }

If I am not mistaken, moving the localized strings to a different file will break both GTK and EFL (EFL just moved to JS controls and actually they use the Apple ones until they redesign them). I think you need to add the localized string file also for GTK and EFL in order not to break those ports.

[Xabier Rodríguez Calvar]

(In reply to comment #11)
> (From update of attachment 229151 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=229151&action=review
> 
> > Source/WebCore/rendering/RenderThemeMac.mm:245
> > -    if (m_mediaControlsScript.isEmpty())
> > -        m_mediaControlsScript = [NSString stringWithContentsOfFile:[[NSBundle bundleForClass:[WebCoreRenderThemeBundle class]] pathForResource:@"mediaControlsApple" ofType:@"js"] encoding:NSUTF8StringEncoding error:nil];
> > +    if (m_mediaControlsScript.isEmpty()) {
> > +        StringBuilder scriptBuilder;
> > +        scriptBuilder.append([NSString stringWithContentsOfFile:[[NSBundle bundleForClass:[WebCoreRenderThemeBundle class]] pathForResource:@"mediaControlsLocalizedStrings" ofType:@"js"] encoding:NSUTF8StringEncoding error:nil]);
> > +        scriptBuilder.append([NSString stringWithContentsOfFile:[[NSBundle bundleForClass:[WebCoreRenderThemeBundle class]] pathForResource:@"mediaControlsApple" ofType:@"js"] encoding:NSUTF8StringEncoding error:nil]);
> > +        m_mediaControlsScript = scriptBuilder.toString();
> > +    }
> 
> If I am not mistaken, moving the localized strings to a different file will break both GTK and EFL (EFL just moved to JS controls and actually they use the Apple ones until they redesign them). I think you need to add the localized string file also for GTK and EFL in order not to break those ports.

And actually, it did.

http://build.webkit.org/results/GTK%20Linux%2064-bit%20Release/r167157%20%2846401%29/accessibility/media-element-pretty-diff.html

[Brent Fulgham]

(In reply to comment #12)
> (In reply to comment #11)

> > 
> > If I am not mistaken, moving the localized strings to a different file will break both GTK and EFL (EFL just moved to JS controls and actually they use the Apple ones until they redesign them). I think you need to add the localized string file also for GTK and EFL in order not to break those ports.
> 
> And actually, it did.
> 
> http://build.webkit.org/results/GTK%20Linux%2064-bit%20Release/r167157%20%2846401%29/accessibility/media-element-pretty-diff.html

Martin Robinson and I already saw this, and he has an approved patch waiting to land as soon as EWS showed successful runs.

[Xabier Rodríguez Calvar]

(In reply to comment #13)
> (In reply to comment #12)
> > (In reply to comment #11)
> 
> > > 
> > > If I am not mistaken, moving the localized strings to a different file will break both GTK and EFL (EFL just moved to JS controls and actually they use the Apple ones until they redesign them). I think you need to add the localized string file also for GTK and EFL in order not to break those ports.
> > 
> > And actually, it did.
> > 
> > http://build.webkit.org/results/GTK%20Linux%2064-bit%20Release/r167157%20%2846401%29/accessibility/media-element-pretty-diff.html
> 
> Martin Robinson and I already saw this, and he has an approved patch waiting to land as soon as EWS showed successful runs.

EFL could be broken too, see bug 120956