aria-sort should fire attributechange

RESOLVED FIXED in mozilla6

Status

()

Core
Disability Access APIs
RESOLVED FIXED
7 years ago
6 years ago

People

(Reporter: davidb, Assigned: davidb)

Tracking

(Blocks: 2 bugs, {dev-doc-complete})

unspecified
mozilla6
dev-doc-complete
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Request from the wild. Patch coming.
Created attachment 518482 [details] [diff] [review]
patch
Attachment #518482 - Flags: review?(surkov.alexander)
Note I created a separate test file for this instead of merging attr change tests because of our coalescence :)

Comment 3

7 years ago
Comment on attachment 518482 [details] [diff] [review]
patch


> 		test_aria_hidden.html \
> 		test_aria_menu.html \
>+		test_aria_sort.html \

perhaps you should combine with aria_hidden and name it test_objattrchange.html?
Attachment #518482 - Flags: review?(surkov.alexander) → review+

Updated

7 years ago
Blocks: 632301
(In reply to comment #3)
> Comment on attachment 518482 [details] [diff] [review]
> patch
> 
> 
> > 		test_aria_hidden.html \
> > 		test_aria_menu.html \
> >+		test_aria_sort.html \
> 
> perhaps you should combine with aria_hidden and name it
> test_objattrchange.html?

I tried exactly that (same name even) locally before posting the patch. I wasn't getting the both events, which I assumed to be our coalescence? (comment 2)

Comment 5

7 years ago
If you didn't do that in separate invokers and for the same node then yes. Just do it in different invokers.

Comment 6

7 years ago
David, is this patch ready taking into account comments 3-5 concerning to mochitest organization?
(In reply to comment #5)
> If you didn't do that in separate invokers and for the same node then yes. Just
> do it in different invokers.

I had separate invokers. Strange (maybe I forgot a 'var' or something). I'd prefer to land the patch as is, with a follow up; given priorities.

Comment 8

7 years ago
If we are both agree it makes testsuite cleaner then we should get it fixed. If you don't have time to finish it then you should ask somebody. The fix should be easy.
Created attachment 524279 [details] [diff] [review]
patch to land (combined tests)

Found some time on the plane on Sunday :)
Attachment #518482 - Attachment is obsolete: true
Attachment #524279 - Flags: review+
Attachment #524279 - Flags: review+

Comment 10

7 years ago
Comment on attachment 524279 [details] [diff] [review]
patch to land (combined tests)


>+      testAttrs("sort", {"sort" : "ascending"}, true);
>       testAttrs("sortAscending", {"sort" : "ascending"}, true);
>+
>+  <div id="sort" role="columnheader" aria-sort="ascending">a column header</div>

why do you need this if you have
<div id="sortAscending" role="columnheader" aria-sort="ascending"></div>

>+      this.getID = function updateSort_getID()
>+      {
>+        return "aria-sort for " + aID + " " + aSort;

perhaps "change aria-sort on " + aSort + " for " + prettyName(aID)?

>+  <div id="sortable" role="columnheader" aria-sort"none">a column</div>

aria-sort="none"

Comment 11

6 years ago
Ping? This is needed for full accessibility of the new version of Y! Mail.
(In reply to comment #10)
> Comment on attachment 524279 [details] [diff] [review] [review]
> patch to land (combined tests)
> 
> 
> >+      testAttrs("sort", {"sort" : "ascending"}, true);
> >       testAttrs("sortAscending", {"sort" : "ascending"}, true);
> >+
> >+  <div id="sort" role="columnheader" aria-sort="ascending">a column header</div>
> 
> why do you need this if you have
> <div id="sortAscending" role="columnheader" aria-sort="ascending"></div>

Good catch, it is redundant. I've removed it locally.

> >+  <div id="sortable" role="columnheader" aria-sort"none">a column</div>
> 
> aria-sort="none"

Sure, updated locally.

Looking for a clean try run before landing.
Landed on central: http://hg.mozilla.org/mozilla-central/rev/43fa778d3f1b
Should see this in FF6.
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla6

Updated

6 years ago
Blocks: 658181
Keywords: dev-doc-needed
Mentioned on Firefox 6 for developers.
Keywords: dev-doc-needed → dev-doc-complete
You need to log in before you can comment on or make changes to this bug.