Summary: | Regression: media controls and status messages are no longer localized | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Jer Noble <jer.noble> | ||||
Component: | New Bugs | Assignee: | Jer Noble <jer.noble> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | bfulgham, calvaris, commit-queue, eric.carlson, esprehn+autocc, glenn, jcraig, jer.noble, jonlee, kondapallykalyan, philipj, sergio, webkit-bug-importer | ||||
Priority: | P2 | Keywords: | InRadar | ||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Bug Depends on: | 123749 | ||||||
Bug Blocks: | 131569, 131570 | ||||||
Attachments: |
|
Description
Jer Noble
2013-09-07 09:54:06 PDT
related to bug 121990 Blocking against bug 123749 because I've got more strings coming in that one. Created attachment 229151 [details]
Patch
Comment on attachment 229151 [details]
Patch
Nice! r=me.
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! Committed r167145: <http://trac.webkit.org/changeset/167145> (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? (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##" Curl quotes landed in follow-up: r167147 <http://trac.webkit.org/changeset/167147> 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. (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 (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. (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 |