Bug 27941 - HTMLInputElement of type range is not controllable by assistive technologies
Summary: HTMLInputElement of type range is not controllable by assistive technologies
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Accessibility (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Eric Carlson
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-08-03 07:53 PDT by Eric Carlson
Modified: 2009-08-03 13:43 PDT (History)
3 users (show)

See Also:


Attachments
proposed patch (39.39 KB, patch)
2009-08-03 08:17 PDT, Eric Carlson
eric: 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-03 07:53:07 PDT
<input type=range> should be identified as a slider and should be controllable by platform assistive technology.
Comment 1 Eric Carlson 2009-08-03 08:17:41 PDT
Created attachment 33976 [details]
proposed patch
Comment 2 Eric Seidel (no email) 2009-08-03 09:42:41 PDT
Comment on attachment 33976 [details]
proposed patch

Seems this should be in some sort of method on HTMLInputElement instead:
8     // Fire change event manually, as RenderSlider::setValueForPosition does.
 109     input->dispatchFormControlChangeEvent();

Null check needed?
ntRect AccessibilitySliderThumb::elementRect() const
 139 {
 140     IntRect intRect = static_cast<RenderSlider*>(m_parentSlider->renderer())->thumbRect();
 141     FloatQuad floatQuad = m_parentSlider->renderer()->localToAbsoluteQuad(FloatRect(intRect));
 142 

Eventually we should write a helper to do this:
 296     thumbRect.setWidth(thumb->style()->width().calcMinValue(contentWidth()));
 297     thumbRect.setHeight(thumb->style()->height().calcMinValue(contentHeight()));
 I feel like we do that often all over the code. :)

Same is true with the centering algorithms you use in the next line.  :)  (Not for this patch, just noting that eventually we'll need more Rect/Point math code.)

I find AccessibilityUIElement.h is a confusing name for this DRT-only class.

It's sad that everyone replicates the js-testing harness in their tests. :(  You know about fast/js/resources/, TEMPLATE.html, and make-js-tests right?

In general this looks fine though.
Comment 3 Eric Carlson 2009-08-03 10:08:40 PDT
http://trac.webkit.org/changeset/46720
Comment 4 Eric Seidel (no email) 2009-08-03 13:00:33 PDT
I was slightly surprised to see no response to my comments. ;)
Comment 5 Eric Carlson 2009-08-03 13:37:20 PDT
Oh, I took the final line in your comment literally, sorry.

For completeness: I added the NULL check, and I did not know about TEMPLATE.html or make-js-tests (but do now, thanks!).
Comment 6 Eric Seidel (no email) 2009-08-03 13:43:25 PDT
(In reply to comment #5)
> Oh, I took the final line in your comment literally, sorry.

It was definitely intended literarily!  So you took it as intended. :)

> For completeness: I added the NULL check, and I did not know about
> TEMPLATE.html or make-js-tests (but do now, thanks!).

Excellent.  Mostly I wanted to hear that you had added the null check when landing.  Thanks!