555728 - Fire a11y event based on HTML5 constraint validation

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[David Bolter [:davidb] (NeedInfo me for attention)]

Attachment: a start (no tests, and we need to check exposure via obj attr)

This is a potential enhancement TODO for when bug 345624 lands.

Comment 1

[David Bolter [:davidb] (NeedInfo me for attention)]

Thanks for the heads up Mounir.

Comment 2

[alexander :surkov (:asurkov)]

We need to set STATE_INVALID as Marco pointed in bug 345624 comment #6.

So whenever @pattern attribute is pointed or proper @type of input control is used (like email type) then we need to manage invalid state and fire state attribute change.

Comment 3

[alexander :surkov (:asurkov)]

form elements use :valid/:invalid CSS3 pseudo-classes starting from bug 558788, we should set STATE_INVALID according to these pseudo-classes.

Comment 4

[David Bolter [:davidb] (NeedInfo me for attention)]

Attachment: WIP

I think this is probably all we need for the event, but I need to add tests. Note bug 601205 dealt with exposing the invalid state which we get nicely from Mounir's intrinsic state.

Comment 5

[David Bolter [:davidb] (NeedInfo me for attention)]

Comment on attachment 482932 [details] [diff] [review]
WIP

Might as well get an early review for this part.

Comment 6

[alexander :surkov (:asurkov)]

I wonder whether we need an event for state invalidate unsetting, mochitests are needed, otherwise looks good.

Comment 7

[David Bolter [:davidb] (NeedInfo me for attention)]

I think for now we should just fire the one side of the event. I'll add tests and repost when I get back to this one.

Comment 8

[David Bolter [:davidb] (NeedInfo me for attention)]

Attachment: patch

Test passes :)

Comment 9

[alexander :surkov (:asurkov)]

Comment on attachment 483573 [details] [diff] [review]
patch


>+  if (aStateMask & NS_EVENT_STATE_INVALID) {
>+    nsRefPtr<AccEvent> event =
>+    new AccStateChangeEvent(aContent1, nsIAccessibleStates::STATE_INVALID,
>+                            PR_FALSE, PR_TRUE);

nit: make correct indent here

>+    FireDelayedAccessibleEvent(event);
>+  }
>+
>   if (0 == (aStateMask & NS_EVENT_STATE_CHECKED)) {
>     return;
>   }
> 
>   nsHTMLSelectOptionAccessible::SelectionChangedIfOption(aContent1);
>   nsHTMLSelectOptionAccessible::SelectionChangedIfOption(aContent2);

nit: put this under if statement, that should be nicer

>+    function invalidInput(aNodeOrID, bExpand)

b expand? anyway you don't use it

>+    {
>+      this.DOMNode = getNode(aNodeOrID);
>+
>+      this.invoke = function invalid_invoke() {

invalidInput_invoke please

>+        // Note: this should fire an EVENT_STATE_CHANGE
>+        this.DOMNode.value = "I am too long";
>+      };
>+
>+      this.check = function expand_check() {

expand_check?

>+        testStates(aNodeOrID, STATE_INVALID);
>+      };
>+
>+      this.getID = function changeValue_getID() {
>+        return prettyName(aNodeOrID) + " aria-expanded changed";

again copy/paste

>+      // invalid state change

nit: first capital letter please

r=me with these fixed

Comment 10

[David Bolter [:davidb] (NeedInfo me for attention)]

Attachment: patch 2

Comment 11

[Marco Zehe (:MarcoZ)]

Comment on attachment 483957 [details] [diff] [review]
patch 2

r=me

Comment 12

[alexander :surkov (:asurkov)]

(In reply to comment #9)
> 
> >+    FireDelayedAccessibleEvent(event);
> >+  }
> >+
> >   if (0 == (aStateMask & NS_EVENT_STATE_CHECKED)) {
> >     return;
> >   }
> > 
> >   nsHTMLSelectOptionAccessible::SelectionChangedIfOption(aContent1);
> >   nsHTMLSelectOptionAccessible::SelectionChangedIfOption(aContent2);
> 
> nit: put this under if statement, that should be nicer

patch2 doesn't address this comment.

Comment 13

[David Bolter [:davidb] (NeedInfo me for attention)]

I've addressed it locally. The delay in pushing this is due to a timeout in the statechange test which I need to fix.

Comment 14

[David Bolter [:davidb] (NeedInfo me for attention)]

http://hg.mozilla.org/mozilla-central/rev/741e9889fbde