Closed Bug 1152454 Opened 9 years ago Closed 9 years ago

[AccessFu]: announce value and name change events if they happen within an aria-live subtree.

Categories

(Core :: Disability Access APIs, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla40
Tracking Status
firefox40 --- fixed

People

(Reporter: yzen, Assigned: bugzilla)

Details

Attachments

(1 file, 6 obsolete files)

If the name or value change events happen within the aria-live subtree, we need to present then similar to how we already do for text change.
Assignee: nobody → bugzilla
Attachment #8593584 - Flags: feedback?(yzenevich)
Attached patch Updated prelim changes (obsolete) — Splinter Review
I fixed the role on my test markup so that aria-valuetext now has an effect on the aAccessible's value.  All tests are passing now.
Attachment #8593584 - Attachment is obsolete: true
Attachment #8593584 - Flags: feedback?(yzenevich)
Attachment #8593662 - Flags: feedback?(yzenevich)
Comment on attachment 8593662 [details] [diff] [review]
Updated prelim changes

Tried to link to the specific commit this time instead of the Comparison Page... Bugzilla apparently still doesn't like it as a valid patch URL.  In any case, just browse to the link to see the changed files and/or make comments.
Heh. It is confusing, but for gecko patches, we attach the actual patch to bugs. Here are some tools that make it easy:

https://github.com/mozilla/moz-git-tools
(In reply to Eitan Isaacson [:eeejay] from comment #4)
> Heh. It is confusing, but for gecko patches, we attach the actual patch to
> bugs. Here are some tools that make it easy:
> 
> https://github.com/mozilla/moz-git-tools

Thanks, yeah, I had started reading through a few pages on how to convert Git to Mercurial but since the patch was still largely a work in progress this afternoon, Yura and I were just looking to get a quick snapshot up somewhere so he could better answer some of the questions I had.  I'll finish reading through those docs though and try to get everything proper for the official submission.
Attachment #8593662 - Flags: feedback?(yzenevich)
Attached file Update 2: Prelim changes (obsolete) —
Hey Yura,

Here's the latest commit with comments addressed.
Attachment #8593662 - Attachment is obsolete: true
Attachment #8594003 - Flags: feedback?(yzenevich)
Comment on attachment 8594003 [details]
Update 2: Prelim changes

Looks good, feel free to mark me for r?
Attachment #8594003 - Flags: feedback?(yzenevich) → feedback+
Here's the HG patch with comments addressed.  Followed the instructions for patch conversion so it should be good to go, but let me know if there are any formatting issues, etc.
Attachment #8594003 - Attachment is obsolete: true
Attachment #8594105 - Flags: a11y-review?(yzenevich)
Noticed another nit.
Attachment #8594105 - Attachment is obsolete: true
Attachment #8594105 - Flags: a11y-review?(yzenevich)
Attachment #8594115 - Flags: review?(yzenevich)
Comment on attachment 8594115 [details] [diff] [review]
0001-Preliminary-changes-to-make-liveregions-responsive-t.patch

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

You also need to update the test_content_integration in terms of traversal from beginning to the end of the document. Prior to this patch, the last item was an apple button (see line 60) now we can actually go 1 step further and hit the progress bar. You should check for something like:

[ContentMessages.simpleMoveNext,
  new ExpectedCursorChange(['slider', '0', {'string': 'slider'}])]

there, and then correctly step backwards as well. There's also a to-do check for moveLast (that is not working yet) that expects an apple button, that should be updated as well.

Flip the review back to me once you patch it up. The patch itself is great!

::: accessible/jsat/Presentation.jsm
@@ +546,4 @@
>        type: this.type,
>        details: {
>          eventType: 'value-change',
> +        data: [aAccessible.value],

Let's not wrap this in []. Instead update line 632 of jsatcommon.js to not wrap the data in an array. I think that should resolve your issue.
Attachment #8594115 - Flags: review?(yzenevich)
Okay, made the changes and should be set.
Attachment #8594115 - Attachment is obsolete: true
Attachment #8594897 - Flags: review?(yzenevich)
Comment on attachment 8594897 [details] [diff] [review]
0001-Made-liveregions-responsive-to-name-value-change-eve.patch

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

Looks good. Once you're certain the patch is formatted correctly, just add checkin-needed keyword. Thanks, Ross!
Attachment #8594897 - Flags: review?(yzenevich) → review+
Updated commit message and format
Attachment #8594897 - Attachment is obsolete: true
Attachment #8595553 - Flags: review?(yzenevich)
Comment on attachment 8595553 [details] [diff] [review]
0001-Bug-1152454-Made-liveregions-responsive-to-name-valu.patch

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

carrying over r+
Attachment #8595553 - Flags: review?(yzenevich) → review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/940168142fdb
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
You need to log in before you can comment on or make changes to this bug.