Bug 96076

Summary:

AX: ARIA spin button should support range value attributes

Product:

WebKit

Reporter:

Dominic Mazzoni <dmazzoni>

Component:

Accessibility

Assignee:

Dominic Mazzoni <dmazzoni>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

abarth, aboxhall, cfleizach, dglazkov, donggwan.kim, fishd, jamesr, peter+ews, tkent, tkent+wkapi, webkit.review.bot, yosin

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

Bug Blocks:

96032

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	none

Dominic Mazzoni

Reported 2012-09-07 00:22:54 PDT

According to the ARIA spec, an ARIA spin button should use aria-valuenow, etc. for its value.

Attachments
Patch (13.93 KB, patch) 2012-09-07 00:45 PDT, Dominic Mazzoni	no flags	Details Formatted Diff Diff
Patch (16.69 KB, patch) 2012-09-07 01:11 PDT, Dominic Mazzoni	no flags	Details Formatted Diff Diff
Patch (16.68 KB, patch) 2012-09-07 01:53 PDT, Dominic Mazzoni	no flags	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Dominic Mazzoni

Comment 1 2012-09-07 00:45:11 PDT

Created attachment 162693 [details] Patch

chris fleizach

Comment 2 2012-09-07 00:48:55 PDT

Comment on attachment 162693 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=162693&action=review > Source/WebCore/accessibility/AccessibilityRenderObject.cpp:1410 > + if (isInputTag) is this part of the same bug? > Source/WebCore/accessibility/AccessibilityRenderObject.cpp:3155 > + if (m_ariaRole != UnknownRole) is this part of the same bug? > Source/WebCore/accessibility/AccessibilityRenderObject.cpp:3394 > + case ScrollBarRole: should you have removed scrollbar role? > Source/WebCore/accessibility/AccessibilityRenderObject.h:305 > + bool isAriaRange() const; should be isARIARange() > Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapper.mm:2002 > + if (m_object->isProgressIndicator() || m_object->isSlider() || m_object->isScrollbar() || m_object->isSpinButton()) Looks like we should probably have a supportsRangeValue() method

Dominic Mazzoni

Comment 3 2012-09-07 01:11:09 PDT

Created attachment 162698 [details] Patch

Dominic Mazzoni

Comment 4 2012-09-07 01:11:43 PDT

Comment on attachment 162693 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=162693&action=review >> Source/WebCore/accessibility/AccessibilityRenderObject.cpp:1410 >> + if (isInputTag) > > is this part of the same bug? I can move this to a separate bug. >> Source/WebCore/accessibility/AccessibilityRenderObject.cpp:3155 >> + if (m_ariaRole != UnknownRole) > > is this part of the same bug? Yes. Without this, a spin button's inner text becomes its title, but I think it generalizes to any ARIA role other than the ones that already explicitly get their title from inner text (like button). This is covered by the test, as well. >> Source/WebCore/accessibility/AccessibilityRenderObject.cpp:3394 >> + case ScrollBarRole: > > should you have removed scrollbar role? I wasn't sure. I'll leave it out. >> Source/WebCore/accessibility/AccessibilityRenderObject.h:305 >> + bool isAriaRange() const; > > should be isARIARange() Done. >> Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapper.mm:2002 >> + if (m_object->isProgressIndicator() || m_object->isSlider() || m_object->isScrollbar() || m_object->isSpinButton()) > > Looks like we should probably have a supportsRangeValue() method Good idea, done.

WebKit Review Bot

Comment 5 2012-09-07 01:14:59 PDT

Please wait for approval from abarth@webkit.org, dglazkov@chromium.org, fishd@chromium.org, jamesr@chromium.org or tkent@chromium.org before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.

Kent Tamura

Comment 6 2012-09-07 01:16:39 PDT

Comment on attachment 162698 [details] Patch OK for Chromium WebKit API change.

Peter Beverloo (cr-android ews)

Comment 7 2012-09-07 01:49:13 PDT

Comment on attachment 162698 [details] Patch Attachment 162698 [details] did not pass cr-android-ews (chromium-android): Output: http://queues.webkit.org/results/13785354

Dominic Mazzoni

Comment 8 2012-09-07 01:53:05 PDT

Created attachment 162712 [details] Patch