role="spinbutton" components are not required to have accompanying buttons to adjust the value, so they should be adjustable on their own. Example component can be found on: https://www.w3.org/WAI/ARIA/apg/patterns/spinbutton/
<rdar://problem/98426197>
Created attachment 461519 [details] Patch
Created attachment 461520 [details] Patch
(In reply to Tyler Wilcock from comment #3) > Created attachment 461520 [details] > Patch Looks good. --- a/LayoutTests/accessibility/spinbutton-increment-decrement.html +++ a/LayoutTests/accessibility/spinbutton-increment-decrement.html Should we also test that the spin button exposes the right value after increment/decrement?
Created attachment 461531 [details] Patch
Comment on attachment 461531 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=461531&action=review > Source/WebCore/accessibility/AccessibilityNodeObject.cpp:1205 > +void AccessibilityNodeObject::alterStepperValue(StepAction stepAction) How about "range" which the superclass of slider, scrollbar, and spinbutton. Note: ARIA spinbutton = Mac "stepper" > Source/WebCore/accessibility/AccessibilityNodeObject.cpp:1207 > + if (roleValue() != AccessibilityRole::Slider && roleValue() != AccessibilityRole::SpinButton) scrollbar too, right? > Source/WebCore/accessibility/AccessibilityNodeObject.cpp:1217 > if (!getAttribute(stepAttr).isEmpty()) > - changeValueByStep(increase); > + changeValueByStep(stepAction); > else > - changeValueByPercent(increase ? 5 : -5); > + changeValueByPercent(stepAction == StepAction::Increment ? 5 : -5); I think direct value changes should only happen on native form elements, never on custom ARIA fields. Does this cover all the HTML5 stepper types like <input type="number">? There are about 2 dozen input types, and several are steppers. The layout test should probably cover these too... I don't recall if the stepAttr conditional will catch all. Does this also catch the default step value when the attribute is missing like `input[type=slider]:not([step])`?
> several are steppers. Looks likeWebKit, Chrome, and Firefox implement type=number as the only stepper, too. I recalled there were more. 🤷
Created attachment 461539 [details] Patch
(In reply to Tyler Wilcock from comment #8) > Created attachment 461539 [details] > Patch --- a/Source/WebCore/accessibility/AccessibilityNodeObject.cpp +++ a/Source/WebCore/accessibility/AccessibilityNodeObject.cpp void AccessibilityNodeObject::changeValueByPercent(float percentChange) @@ -1321,7 +1325,7 @@ void AccessibilityNodeObject::changeValueByPercent(float percentChange) step = std::abs(percentChange) * (1 / percentChange); value += step; - setNodeValue(percentChange > 0, value); + setNodeValue(percentChange > 0 ? StepAction::Increment : StepAction::Decrement, value); } This function is missing some early returns and sanity checks like percentCahnge == 0, and value between min and max... Since you're touching this, could you please add them? --- a/LayoutTests/platform/ios/accessibility/spinbutton-increment-decrement-expected.txt +++ a/LayoutTests/platform/ios/accessibility/spinbutton-increment-decrement-expected.txt Interesting that the value for MacOS and iOS differ in type? int vs float?
Created attachment 461555 [details] Patch
(In reply to Andres Gonzalez from comment #9) > (In reply to Tyler Wilcock from comment #8) > > Created attachment 461539 [details] > > Patch > > --- a/Source/WebCore/accessibility/AccessibilityNodeObject.cpp > +++ a/Source/WebCore/accessibility/AccessibilityNodeObject.cpp > > void AccessibilityNodeObject::changeValueByPercent(float percentChange) > @@ -1321,7 +1325,7 @@ void > AccessibilityNodeObject::changeValueByPercent(float percentChange) > step = std::abs(percentChange) * (1 / percentChange); > > value += step; > - setNodeValue(percentChange > 0, value); > + setNodeValue(percentChange > 0 ? StepAction::Increment : > StepAction::Decrement, value); > } > > This function is missing some early returns and sanity checks like > percentCahnge == 0, and value between min and max... Since you're touching > this, could you please add them? I added the check for !percentChange. Investigated the prevention of setting values outside the min and max, but that will be a little more involved (because we can't just do it in this one function, or need to centralize it somewhere else) and will require it's own layout test, so I think it will be best to save that one for a separate patch. > --- > a/LayoutTests/platform/ios/accessibility/spinbutton-increment-decrement- > expected.txt > +++ > a/LayoutTests/platform/ios/accessibility/spinbutton-increment-decrement- > expected.txt > Interesting that the value for MacOS and iOS differ in type? int vs float? Yeah, it's unfortunate...we should make these the same in a later patch.
Committed 253403@main (9a87db7810f8): <https://commits.webkit.org/253403@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 461555 [details].