Closed Bug 1054454 Opened 10 years ago Closed 10 years ago

[AccessFu] Improve aria-hidden handling in AccessFu.

Categories

(Core :: Disability Access APIs, defect)

All
Gonk (Firefox OS)
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla34

People

(Reporter: yzen, Assigned: yzen)

References

Details

Attachments

(1 file, 2 obsolete files)

We need to listen and handle properly aria-hidden attribute change events. Also need to take into account aria-hidden when dealing with automove.
Depends on: 1056803
Attached patch 1054454 patch v1 (obsolete) — Splinter Review
Attachment #8476724 - Flags: review?(eitan)
Status: NEW → ASSIGNED
Comment on attachment 8476724 [details] [diff] [review]
1054454 patch v1

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

Overall, good. I am wondering if we can change the case I mentioned below. Also, maybe a test for a automove in a hidden iframe? Don't feel too strongly about that (ie. don't spend too much time on that).

::: accessible/tests/mochitest/jsat/test_content_integration.html
@@ +167,5 @@
> +          }],
> +          [doc.defaultView.ariaHideBack, {
> +            speak: ["wow", {"string": "headingLevel","args": [1]}, "such app"],
> +          }],
> +          [doc.defaultView.ariaHideBack],

Add a comment here of why you are doing this again.

@@ +182,5 @@
> +          [doc.defaultView.ariaHideBack],
> +          [ContentMessages.focusSelector('button#back', false), {
> +            // Must not speak Back button as it is aria-hidden
> +            speak: ["wow", {"string": "headingLevel","args": [1]}, "such app"],
> +          }],

So, in this case the autoMove tries to land on the back button, but it is hidden so it jumps to the next item? That behavior is not ideal. The autoMove should probably be repressed.

So:
1. The focus changed in a hidden sub-doc -> move vc quietly without presenting.
2. The focus changed to a hidden element (or descendant-of) -> repress autoMove.
Attachment #8476724 - Flags: review?(eitan)
(In reply to Eitan Isaacson [:eeejay] from comment #2)
> Comment on attachment 8476724 [details] [diff] [review]
> 1054454 patch v1
> 
> Review of attachment 8476724 [details] [diff] [review]:
> -----------------------------------------------------------------
> @@ +182,5 @@
> > +          [doc.defaultView.ariaHideBack],
> > +          [ContentMessages.focusSelector('button#back', false), {
> > +            // Must not speak Back button as it is aria-hidden
> > +            speak: ["wow", {"string": "headingLevel","args": [1]}, "such app"],
> > +          }],
> 
> So, in this case the autoMove tries to land on the back button, but it is
> hidden so it jumps to the next item? That behavior is not ideal. The
> autoMove should probably be repressed.
> 
> So:
> 1. The focus changed in a hidden sub-doc -> move vc quietly without
> presenting.
> 2. The focus changed to a hidden element (or descendant-of) -> repress
> autoMove.

Opened bug 1057457 for this issue.
Attached patch 1054454 patch v2 (obsolete) — Splinter Review
Added comment and additional tests.
Attachment #8476724 - Attachment is obsolete: true
Attachment #8478301 - Flags: review?(eitan)
Attachment #8478301 - Flags: review?(eitan) → review+
Attached patch 1054454 patch v3Splinter Review
Carrying over the r+ from eeejay. Updated to the latest nsIAccessibleObjectAttributeChanged interface:
attributeName -> changedAttribute.toString()
Attachment #8478301 - Attachment is obsolete: true
Attachment #8479592 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/d20f8a8d7499
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
You need to log in before you can comment on or make changes to this bug.