Fire a11y event based on HTML5 constraint validation

RESOLVED FIXED in mozilla2.0b8

Status

()

defect
RESOLVED FIXED
9 years ago
9 years ago

People

(Reporter: davidb, Assigned: davidb)

Tracking

(Blocks 1 bug)

unspecified
mozilla2.0b8
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 3 obsolete attachments)

This is a potential enhancement TODO for when bug 345624 lands.
Attachment #435644 - Attachment is patch: true
Attachment #435644 - Attachment mime type: application/octet-stream → text/plain
Assignee: nobody → bolterbugz
Thanks for the heads up Mounir.
Depends on: 345624
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.
Summary: Fire state change for oninvalid (from new HTML5 Constraint Validation) → Map HTML5 Constraint Validation to AT API
Depends on: 556013, 345512
form elements use :valid/:invalid CSS3 pseudo-classes starting from bug 558788, we should set STATE_INVALID according to these pseudo-classes.
Depends on: 558788
Posted patch WIP (obsolete) — Splinter Review
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.
Attachment #435644 - Attachment is obsolete: true
Summary: Map HTML5 Constraint Validation to AT API → Fire a11y event based on HTML5 constraint validation
Comment on attachment 482932 [details] [diff] [review]
WIP

Might as well get an early review for this part.
Attachment #482932 - Flags: review?(surkov.alexander)
I wonder whether we need an event for state invalidate unsetting, mochitests are needed, otherwise looks good.
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.
Attachment #482932 - Flags: review?(surkov.alexander)
Posted patch patch (obsolete) — Splinter Review
Test passes :)
Attachment #482932 - Attachment is obsolete: true
Attachment #483573 - Flags: review?(surkov.alexander)
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
Attachment #483573 - Flags: review?(surkov.alexander) → review+
Posted patch patch 2Splinter Review
Attachment #483573 - Attachment is obsolete: true
Attachment #483957 - Flags: review?(marco.zehe)
Attachment #483957 - Flags: approval2.0? → approval2.0+
(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.
I've addressed it locally. The delay in pushing this is due to a timeout in the statechange test which I need to fix.
http://hg.mozilla.org/mozilla-central/rev/741e9889fbde
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b8
You need to log in before you can comment on or make changes to this bug.