Bug 105940 – AX: Chromium needs platform localized strings for media controls.

• Component: Accessibility
• Status: RESOLVED FIXED

[David Tseng]

Chrome uses its own infrastructure to localize strings within webkit and these strings are not localized (or even exist) in Chrome. Thus this results in the following bug:
- play any mp3 file via a link in Chrome. (note must be Google Chrome).
- inspect any button with an a11y tool on your platform.

result:
no labels.

This hampers the usability of any media control and has been a source of many user complaints.

[David Tseng]

Created attachment 181057 [details]
Patch

[WebKit Review Bot]

Please wait for approval from [e-mail], [e-mail], [e-mail], [e-mail] or [e-mail] before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.

[Tony Chang]

Comment on attachment 181057 [details]
Patch

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

Seems fine, just fix the ChangeLog.

> Source/Platform/ChangeLog:1
> [phone]  My Name  <my@email>

Please put your name and email here. I think you can set user.name and user.email in git to have this automatically filled in.

> Source/WebKit/chromium/ChangeLog:1
> [phone]  My Name  <my@email>

Please put your name and email here.

> Source/WebKit/chromium/ChangeLog:18
> [phone]  My Name  <my@email>
> +
> +        Need a short description (OOPS!).
> +        Need the bug URL (OOPS!).
> +
> +        Reviewed by NOBODY (OOPS!).

Looks like this got duplicated.

[David Tseng]

Created attachment 181067 [details]
Patch

[Silvia Pfeiffer]

FWIW: LGTM

Thanks for taking this on - it's been on my plate for a while, but I never got the chance to look at it. Seems simpler than I thought!

[Tony Chang]

Comment on attachment 181067 [details]
Patch

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

> Source/Platform/chromium/public/WebLocalizedString.h:55
> +        AXMediaDefault,
> +        AXMediaAudioElement,
> +        AXMediaVideoElement,
> +        AXMediaMuteButton,
> +        AXMediaUnMuteButton,
> +        AXMediaPlayButton,
> +        AXMediaPauseButton,

Please sort these.

> Source/WebKit/chromium/src/LocalizedStrings.cpp:462
> +
> +    return query(WebLocalizedString::AXMediaDefault);

Should we ASSERT_NOT_REACHED() here? That's what WebCore/platform/LocalizedStrings.cpp does.

[David Tseng]

Created attachment 181089 [details]
Patch

[David Tseng]

(In reply to comment #6)
> (From update of attachment 181067 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=181067&action=review
> 
> > Source/Platform/chromium/public/WebLocalizedString.h:55
> > +        AXMediaDefault,
> > +        AXMediaAudioElement,
> > +        AXMediaVideoElement,
> > +        AXMediaMuteButton,
> > +        AXMediaUnMuteButton,
> > +        AXMediaPlayButton,
> > +        AXMediaPauseButton,
> 
> Please sort these.

Done
> 
> > Source/WebKit/chromium/src/LocalizedStrings.cpp:462
> > +
> > +    return query(WebLocalizedString::AXMediaDefault);
> 
> Should we ASSERT_NOT_REACHED() here? That's what WebCore/platform/LocalizedStrings.cpp does.

Done (as well as for localizedHelpText); (I went as far as to include the specific check for the ControlsPanel, but not the exception for the enter full screen button help since it seems odd for webkit to make such a singular exception.

[Tony Chang]

Comment on attachment 181089 [details]
Patch

Note that the ews and commit queue bots don't run in debug so if the assert triggers, we probably won't see it until the patch lands.

[WebKit Review Bot]

Comment on attachment 181089 [details]
Patch

Clearing flags on attachment: 181089

Committed r138680: <http://trac.webkit.org/changeset/138680>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.