Bug 28081 - <video> and <audio> controller should be accessible
: <video> and <audio> controller should be accessible
Status: RESOLVED FIXED
Product: WebKit
Classification: Unclassified
Component: Accessibility
: 528+ (Nightly build)
: All All
: P2 Normal
Assigned To: Eric Carlson
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2009-08-07 13:14 PDT by Eric Carlson
Modified: 2009-08-25 16:29 PDT (History)
1 user (show)

See Also:


Attachments
proposed patch (83.81 KB, patch)
2009-08-07 15:14 PDT, Eric Carlson
eric: review-
Details | Formatted Diff | Diff
revised patch (122.97 KB, patch)
2009-08-21 09:03 PDT, Eric Carlson
oliver: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Carlson 2009-08-07 13:14:22 PDT
Built-in <audio> and <video> controls do not work well with accessibility: controls are not in a logic tab order, all buttons names are reported as "button", the timeline slider values are reported as percentages, etc.
Comment 1 Eric Carlson 2009-08-07 15:14:00 PDT
Created attachment 34335 [details]
proposed patch
Comment 2 Eric Carlson 2009-08-10 14:28:38 PDT
<rdar://7065736 >
Comment 3 Eric Carlson 2009-08-12 09:25:36 PDT
http://trac.webkit.org/changeset/47110
Comment 4 Kenneth Rohde Christiansen 2009-08-12 11:12:47 PDT
This patch seem to have broken the Qt build:

failed
123 test cases (2%) had incorrect layout
3031 test cases (58%) crashed
Comment 5 Eric Seidel 2009-08-12 16:01:37 PDT
Comment on attachment 34335 [details]
proposed patch

Was rolled out in:
http://trac.webkit.org/changeset/47140

failing tests were skipped in:
http://trac.webkit.org/changeset/47128
Comment 6 Eric Carlson 2009-08-21 09:03:35 PDT
Created attachment 38374 [details]
revised patch

This is a minor rework of the patch reviewed last week. New this time are updated layout test results for Leopard and Windows, and some gtk localized strings I missed last time.
Comment 7 Oliver Hunt 2009-08-24 11:18:38 PDT
Comment on attachment 38374 [details]
revised patch


> +String AccessibilityMediaTimeline::valueDescription() const
> +{
> +    float time = static_cast<HTMLInputElement*>(m_renderer->node())->value().toFloat();
What is the guarantee that our renderer is an input element?


>  MediaControlMuteButtonElement::MediaControlMuteButtonElement(Document* document, HTMLMediaElement* element)
> -    : MediaControlInputElement(document, MEDIA_CONTROLS_MUTE_BUTTON, "button", element, element->muted() ? MediaUnMuteButton : MediaMuteButton)
> +    :: MediaControlInputElement(document, MEDIA_CONTROLS_MUTE_BUTTON, "button", element)
>  {
>  }
This change looks bogus (:: vs :)

r+ assuming you fix the bogosity and explain why the above cast is safe
Comment 8 Eric Carlson 2009-08-25 15:41:19 PDT
> > +String AccessibilityMediaTimeline::valueDescription() const
> > +{
> > +    float time = static_cast<HTMLInputElement*>(m_renderer->node())->value().toFloat();
> 
> What is the guarantee that our renderer is an input element?

  AccessibilityMediaTimeline is only created if the node is an input element, but I will add an ASSERT() in any case.


> >  MediaControlMuteButtonElement::MediaControlMuteButtonElement(Document* document, HTMLMediaElement* element)
> > -    : MediaControlInputElement(document, MEDIA_CONTROLS_MUTE_BUTTON, "button", element, element->muted() ? MediaUnMuteButton : MediaMuteButton)
> > +    :: MediaControlInputElement(document, MEDIA_CONTROLS_MUTE_BUTTON, "button", element)
> >  {
> >  }
>
> This change looks bogus (:: vs :)

Indeed!
Comment 9 Eric Carlson 2009-08-25 16:29:19 PDT
http://trac.webkit.org/changeset/47763