960255 - [AccessFu] Adjusting a range <input> does not dispatch a change event

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[Eitan Isaacson [:eeejay]]

This is because we are doing it with a keyboard, and a range widget does not have a defined "activation behavior". So the 'change' event will only happen after a focused range is blurred[1]. With a mouse, the 'change' event will happen after the mouseup event when the user is done dragging the slider.

Some apps depend on the 'change' event to update their state.

1. http://www.whatwg.org/specs/web-apps/current-work/multipage/common-input-element-attributes.html#common-input-element-events

Comment 1

[Eitan Isaacson [:eeejay]]

Attachment: Focus/blur range widget after adjustment to dispatch 'change' event.

This does not seem to have any ill side effects, since the vc is already on the widget (and if it is an embedded widget situation, the vc does not go past the label).

Comment 2

[alexander :surkov (:asurkov)]

Comment on attachment 8360632 [details] [diff] [review]
Focus/blur range widget after adjustment to dispatch 'change' event.

Review of attachment 8360632 [details] [diff] [review]:
-----------------------------------------------------------------

::: accessible/src/jsat/content-script.js
@@ +351,5 @@
>              content.KeyEvent.DOM_VK_DOWN : content.KeyEvent.DOM_VK_UP;
>        evt.initKeyEvent(
>          "keypress", false, true, null, false, false, false, false, keycode, 0);
>        acc.DOMNode.dispatchEvent(evt);
> +      acc.DOMNode.blur();

focus tricks looks as a hack, blur() may result in surprise behavior, if the caller didn't suppose a focus change. On a side note, I would try to rely on a11y API to change the value, accessible actions looks appropriate but not sure whether accessible actions should result in 'change' event since 'change' events is fired after focus goes next.

Comment 3

[Eitan Isaacson [:eeejay]]

(In reply to alexander :surkov from comment #2)
> Comment on attachment 8360632 [details] [diff] [review]
> Focus/blur range widget after adjustment to dispatch 'change' event.
> 
> Review of attachment 8360632 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: accessible/src/jsat/content-script.js
> @@ +351,5 @@
> >              content.KeyEvent.DOM_VK_DOWN : content.KeyEvent.DOM_VK_UP;
> >        evt.initKeyEvent(
> >          "keypress", false, true, null, false, false, false, false, keycode, 0);
> >        acc.DOMNode.dispatchEvent(evt);
> > +      acc.DOMNode.blur();
> 
> focus tricks looks as a hack, blur() may result in surprise behavior, if the
> caller didn't suppose a focus change.

I agree. This is the solution I am offering after I exhausted all other options.

> On a side note, I would try to rely on
> a11y API to change the value, accessible actions looks appropriate but not
> sure whether accessible actions should result in 'change' event since
> 'change' events is fired after focus goes next.

Accessible actions currently don't support this. You mean the nsIAccessbleValue interface? That would work with <input> elements. But it breaks too easily on non-native aria range widgets, since they don't observe aria-valuenow changes.

The most universal way to stepup/stepdown in range widgets is with arrow keys. Which is a sad situation. That is why I have interest in IndieUI[1].

If the a11y API would force a DOM 'change' event when accessible.currentValue was changed, I think it would be worthwhile having a special case for native range widgets to use that interface, and still synthesize keyboard interaction with aria range widgets. But like you say that would be a questionable practice.

1. http://www.w3.org/TR/2013/WD-indie-ui-events-20130122/#UIValueChangeRequestEvent

Comment 4

[alexander :surkov (:asurkov)]

(In reply to Eitan Isaacson [:eeejay] from comment #3)

> > focus tricks looks as a hack, blur() may result in surprise behavior, if the
> > caller didn't suppose a focus change.
> 
> I agree. This is the solution I am offering after I exhausted all other
> options.

so you tried to send fake 'change' event? 

(In reply to Eitan Isaacson [:eeejay] from comment #3)

> > On a side note, I would try to rely on
> > a11y API to change the value, accessible actions looks appropriate but not
> > sure whether accessible actions should result in 'change' event since
> > 'change' events is fired after focus goes next.
> 
> Accessible actions currently don't support this. You mean the
> nsIAccessbleValue interface?

actions interface, implement missed part

> The most universal way to stepup/stepdown in range widgets is with arrow
> keys. Which is a sad situation. That is why I have interest in IndieUI[1].

curious how does IndieUI help here?

Comment 5

[Eitan Isaacson [:eeejay]]

(In reply to alexander :surkov from comment #4)
> (In reply to Eitan Isaacson [:eeejay] from comment #3)
> 
> > > focus tricks looks as a hack, blur() may result in surprise behavior, if the
> > > caller didn't suppose a focus change.
> > 
> > I agree. This is the solution I am offering after I exhausted all other
> > options.
> 
> so you tried to send fake 'change' event? 

I did, and was getting NotSupportedError. But now I tried again, and I made a mistake earlier :)
I tried doing createEvent('change') instead of createEvent('UIEvent'). Oops! New patch coming soon.

Comment 6

[alexander :surkov (:asurkov)]

(In reply to Eitan Isaacson [:eeejay] from comment #5)

> I tried doing createEvent('change') instead of createEvent('UIEvent'). Oops!
> New patch coming soon.

good, not sure if it's an ideal solution though ;)

Comment 7

[Eitan Isaacson [:eeejay]]

(In reply to alexander :surkov from comment #4)
> 
> > The most universal way to stepup/stepdown in range widgets is with arrow
> > keys. Which is a sad situation. That is why I have interest in IndieUI[1].
> 
> curious how does IndieUI help here?

An implementer of an ARIA range widget would listen for UIValueChangeRequestEvent instead of keyboard events directly. Then we could implement actions that emit those events.

Comment 8

[Eitan Isaacson [:eeejay]]

Attachment: Emit 'change' event after adjusting range widgets.

Comment 9

[Yura Zenevich [:yzen] (please use "needinfo")]

Comment on attachment 8364495 [details] [diff] [review]
Emit 'change' event after adjusting range widgets.

Review of attachment 8364495 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with comments addressed.

::: accessible/src/jsat/content-script.js
@@ +344,5 @@
>  function adjustRange(aMessage) {
>    function sendUpDownKey(aAccessible) {
>      let acc = Utils.getEmbeddedControl(aAccessible) || aAccessible;
>      if (acc.DOMNode) {
> +      let elem = acc.DOMNode;

Nit: can we take this declaration outside of the if statement.

@@ +351,5 @@
>          content.KeyEvent.DOM_VK_DOWN : content.KeyEvent.DOM_VK_UP;
>        evt.initKeyEvent(
>          "keypress", false, true, null, false, false, false, false, keycode, 0);
> +      elem.dispatchEvent(evt);
> +      if (elem.tagName === 'INPUT') {

If I understand correctly this is input[type='range'] specific. If so, we also want to check the Utils.getAttributes(acc)['text-input-type'] here for range?

Comment 10

[Eitan Isaacson [:eeejay]]

Attachment: Emit 'change' event after adjusting range widgets.

Followed your suggestions. Also using stepUp/stepDown methods instead of key events on native widgets.

Comment 11

[Eitan Isaacson [:eeejay]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c240a360877a

Comment 12

[Eitan Isaacson [:eeejay]]

Comment on attachment 8365416 [details] [diff] [review]
Emit 'change' event after adjusting range widgets.

I got an RIL r+

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/c240a360877a