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

RESOLVED FIXED in Firefox 40

Status

()

Core
Disability Access APIs
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: yzen, Assigned: Ross)

Tracking

unspecified
mozilla40
Points:
---

Firefox Tracking Flags

(firefox40 fixed)

Details

Attachments

(1 attachment, 6 obsolete attachments)

(Reporter)

Description

3 years ago
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)

Updated

3 years ago
Assignee: nobody → bugzilla
(Assignee)

Comment 1

3 years ago
Created attachment 8593584 [details] [diff] [review]
Git diff for proposed changes (preliminary)
(Reporter)

Updated

3 years ago
Attachment #8593584 - Flags: feedback?(yzenevich)
(Assignee)

Comment 2

3 years ago
Created attachment 8593662 [details] [diff] [review]
Updated prelim changes

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)
(Assignee)

Comment 3

3 years ago
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
(Assignee)

Comment 5

3 years ago
(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.
(Reporter)

Updated

3 years ago
Attachment #8593662 - Flags: feedback?(yzenevich)
(Assignee)

Comment 7

3 years ago
Created attachment 8594003 [details]
Update 2: Prelim changes

Hey Yura,

Here's the latest commit with comments addressed.
Attachment #8593662 - Attachment is obsolete: true
Attachment #8594003 - Flags: feedback?(yzenevich)
(Reporter)

Comment 8

3 years ago
Comment on attachment 8594003 [details]
Update 2: Prelim changes

Looks good, feel free to mark me for r?
Attachment #8594003 - Flags: feedback?(yzenevich) → feedback+
(Assignee)

Comment 9

3 years ago
Created attachment 8594105 [details] [diff] [review]
0001-Preliminary-changes-to-make-liveregions-responsive-t.patch

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)
(Assignee)

Comment 10

3 years ago
Created attachment 8594115 [details] [diff] [review]
0001-Preliminary-changes-to-make-liveregions-responsive-t.patch

Noticed another nit.
Attachment #8594105 - Attachment is obsolete: true
Attachment #8594105 - Flags: a11y-review?(yzenevich)
Attachment #8594115 - Flags: review?(yzenevich)
(Reporter)

Comment 11

3 years ago
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)
(Assignee)

Comment 12

3 years ago
Created attachment 8594897 [details] [diff] [review]
0001-Made-liveregions-responsive-to-name-value-change-eve.patch

Okay, made the changes and should be set.
Attachment #8594115 - Attachment is obsolete: true
Attachment #8594897 - Flags: review?(yzenevich)
(Reporter)

Comment 13

3 years ago
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+
(Assignee)

Comment 14

3 years ago
Created attachment 8595553 [details] [diff] [review]
0001-Bug-1152454-Made-liveregions-responsive-to-name-valu.patch

Updated commit message and format
Attachment #8594897 - Attachment is obsolete: true
Attachment #8595553 - Flags: review?(yzenevich)
(Reporter)

Comment 15

3 years ago
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+
(Assignee)

Updated

3 years ago
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/940168142fdb
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox40: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
You need to log in before you can comment on or make changes to this bug.