Bug 28081

Summary: <video> and <audio> controller should be accessible
Product: WebKit Reporter: Eric Carlson <eric.carlson>
Component: AccessibilityAssignee: Eric Carlson <eric.carlson>
Status: RESOLVED FIXED    
Severity: Normal CC: kenneth
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
proposed patch
eric: review-
revised patch oliver: review+

Eric Carlson
Reported 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.
Attachments
proposed patch (83.81 KB, patch)
2009-08-07 15:14 PDT, Eric Carlson
eric: review-
revised patch (122.97 KB, patch)
2009-08-21 09:03 PDT, Eric Carlson
oliver: review+
Eric Carlson
Comment 1 2009-08-07 15:14:00 PDT
Created attachment 34335 [details] proposed patch
Eric Carlson
Comment 2 2009-08-10 14:28:38 PDT
Eric Carlson
Comment 3 2009-08-12 09:25:36 PDT
Kenneth Rohde Christiansen
Comment 4 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
Eric Seidel (no email)
Comment 5 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
Eric Carlson
Comment 6 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.
Oliver Hunt
Comment 7 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
Eric Carlson
Comment 8 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!
Eric Carlson
Comment 9 2009-08-25 16:29:19 PDT
Note You need to log in before you can comment on or make changes to this bug.