Closed
Bug 614310
Opened 14 years ago
Closed 12 years ago
Map <section> to pane (like role="region")
Categories
(Core :: Disability Access APIs, defect)
Core
Disability Access APIs
Tracking
()
RESOLVED
FIXED
mozilla14
People
(Reporter: davidb, Assigned: capella)
References
(Blocks 1 open bug, )
Details
(Whiteboard: [good first bug][mentor=surkov.alexander@gmail.com][lang=c++])
Attachments
(2 files, 1 obsolete file)
4.18 KB,
patch
|
davidb
:
review+
surkov
:
feedback+
|
Details | Diff | Splinter Review |
4.41 KB,
patch
|
davidb
:
feedback+
|
Details | Diff | Splinter Review |
Currently we expose as a paragraph.
Reporter | ||
Comment 1•14 years ago
|
||
Internally we'll map it to section.
Comment 2•13 years ago
|
||
Ping on this? What's the current spec requirement?
Comment 3•13 years ago
|
||
(In reply to Marco Zehe (:MarcoZ) from comment #2) > Ping on this? What's the current spec requirement? region role ROLE_SYSTEM_PANE IA2_ROLE_SECTION Pane ROLE_PANEL AXGroup, AXRoleDescription="region"
Reporter | ||
Comment 4•13 years ago
|
||
Some notes on <section> http://www.paciellogroup.com/blog/2011/03/html5-accessibility-chops-section-elements/
Comment 5•12 years ago
|
||
David, if you aren't going to work on this bug then please turn it into good-first-bug. It should be easy to fix.
Reporter | ||
Comment 6•12 years ago
|
||
I thought I had a local patch for this but don't see it. Marking GFB.
Assignee: bolterbugz → nobody
Whiteboard: [good first bug][mentor=surkov.alexander@gmail.com]
Comment 7•12 years ago
|
||
steps to fix: 1) extend nsHyperTextAccessible::NativeRole (http://mxr.mozilla.org/mozilla-central/source/accessible/src/html/nsHyperTextAccessible.cpp#135) to handle HTML section element 2) exten nsHyperTextAccessible::GetAttributesInternal (http://mxr.mozilla.org/mozilla-central/source/accessible/src/html/nsHyperTextAccessible.cpp#1246) to expose "region" wai-role attribute 3) add tests into elm/test_landmarks.html
Whiteboard: [good first bug][mentor=surkov.alexander@gmail.com] → [good first bug][mentor=surkov.alexander@gmail.com][lang=c++]
Reporter | ||
Comment 9•12 years ago
|
||
(In reply to alexander :surkov from comment #7) > 2) exten nsHyperTextAccessible::GetAttributesInternal > (http://mxr.mozilla.org/mozilla-central/source/accessible/src/html/ > nsHyperTextAccessible.cpp#1246) to expose "region" wai-role attribute Alexander note that neither region nor section are considered landmarks by ARIA. Do you agree we should drop #2 here?
Comment 10•12 years ago
|
||
(In reply to David Bolter [:davidb] from comment #9) > (In reply to alexander :surkov from comment #7) > > 2) exten nsHyperTextAccessible::GetAttributesInternal > > (http://mxr.mozilla.org/mozilla-central/source/accessible/src/html/ > > nsHyperTextAccessible.cpp#1246) to expose "region" wai-role attribute > > Alexander note that neither region nor section are considered landmarks by > ARIA. Do you agree we should drop #2 here? We expose xml-roles object attribute for all ARIA roles. If mapping of a HTML element should be close to some ARIA role then it makes sense to expose xml-roles too. putting tests into test_landmarks may sound strange so we might want to add new test at attributes/test_xml-roles.html Does it sound good?
Assignee | ||
Comment 12•12 years ago
|
||
Requesting feedback from the mentor ...
Assignee: nobody → markcapella
Status: NEW → ASSIGNED
Attachment #609654 -
Flags: feedback?(surkov.alexander)
Comment 13•12 years ago
|
||
Comment on attachment 609654 [details] [diff] [review] Patch (v1) Review of attachment 609654 [details] [diff] [review]: ----------------------------------------------------------------- having attributes/test_xml-roles.html with tests you added is confusing because you test there tag object attribute and roles maybe it makes sense to rename elm/test_landmarks.html to elm/test_xml-roles.html and add new stuffs
Attachment #609654 -
Flags: review?(dbolter)
Attachment #609654 -
Flags: feedback?(surkov.alexander)
Attachment #609654 -
Flags: feedback+
Reporter | ||
Comment 14•12 years ago
|
||
(In reply to alexander :surkov from comment #10) > (In reply to David Bolter [:davidb] from comment #9) > putting tests into test_landmarks may sound strange so we might want to add > new test at attributes/test_xml-roles.html > > Does it sound good? Yep.
Reporter | ||
Comment 15•12 years ago
|
||
Comment on attachment 609654 [details] [diff] [review] Patch (v1) Review of attachment 609654 [details] [diff] [review]: ----------------------------------------------------------------- r=me with a small change: ::: accessible/tests/mochitest/attributes/test_xml-roles.html @@ +25,5 @@ > + // Some AT may look for this > + testAttrs("section", {"xml-roles" : "region"}, true); > + > + // And some AT may look for this > + testAttrs("section", {"tag" : "SECTION"}, true); OK please put these tests in elm/test_landmarks.html. We'll need to do some restructuring of our tests in a follow up bug. Mark would you like to file that?
Attachment #609654 -
Flags: review?(dbolter) → review+
Assignee | ||
Comment 16•12 years ago
|
||
Ok, I'll move the tests from the new file into existing elm/test_landmarks.html, then file a followup bug ... (saw details on IRQ ...) I'll try to incorporate those portions that are clear ...
Assignee | ||
Comment 17•12 years ago
|
||
Changed as discussed ...
Attachment #609709 -
Flags: feedback?(surkov.alexander)
Reporter | ||
Comment 18•12 years ago
|
||
Comment on attachment 609709 [details] [diff] [review] Patch (v2) Review of attachment 609709 [details] [diff] [review]: ----------------------------------------------------------------- ::: accessible/tests/mochitest/elm/test_landmarks.html @@ +25,5 @@ > testRole("article", ROLE_DOCUMENT); > testRole("aside", ROLE_NOTE); > > testRole("main", ROLE_DOCUMENT); > + testRole("section", ROLE_SECTION); Please add a comment like: // XXX Exception - not a landmark. Move during test reorg.
Assignee | ||
Comment 19•12 years ago
|
||
Added reorg bug refererence.
Attachment #609709 -
Attachment is obsolete: true
Attachment #609811 -
Flags: feedback?(dbolter)
Attachment #609709 -
Flags: feedback?(surkov.alexander)
Reporter | ||
Comment 20•12 years ago
|
||
Comment on attachment 609811 [details] [diff] [review] Patch (v3) Review of attachment 609811 [details] [diff] [review]: ----------------------------------------------------------------- Thanks. (Note my r+ sort of trumps this f+ but f request is a good way to flag me) ::: accessible/tests/mochitest/elm/test_landmarks.html @@ +25,5 @@ > testRole("article", ROLE_DOCUMENT); > testRole("aside", ROLE_NOTE); > > testRole("main", ROLE_DOCUMENT); > + // XXX Exception - not a landmark. Move during test reorg bug739612. Thanks. Can you remove the good first bug whiteboard comment for now? The hard part of that bug will be getting agreement :)
Attachment #609811 -
Flags: feedback?(dbolter) → feedback+
Comment 22•12 years ago
|
||
Please can you set the target milestone when landing on inbound, along the lines of http://blog.bonardo.net/2012/03/23/how-you-can-help-mozilla-inbound-sheriffs-when-pushing :-) https://hg.mozilla.org/mozilla-central/rev/3083df9ca118
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla14
You need to log in
before you can comment on or make changes to this bug.
Description
•