Last Comment Bug 614310 - Map <section> to pane (like role="region")
: Map <section> to pane (like role="region")
Status: RESOLVED FIXED
[good first bug][mentor=surkov.alexan...
:
Product: Core
Classification: Components
Component: Disability Access APIs (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla14
Assigned To: Mark Capella [:capella]
:
:
Mentors:
http://dev.w3.org/html5/html-api-map/...
: 719852 (view as bug list)
Depends on:
Blocks: html5a11y
  Show dependency treegraph
 
Reported: 2010-11-23 10:25 PST by David Bolter [:davidb]
Modified: 2012-03-28 14:00 PDT (History)
7 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch (v1) (4.18 KB, patch)
2012-03-27 01:29 PDT, Mark Capella [:capella]
dbolter: review+
surkov.alexander: feedback+
Details | Diff | Splinter Review
Patch (v2) (4.34 KB, patch)
2012-03-27 07:54 PDT, Mark Capella [:capella]
no flags Details | Diff | Splinter Review
Patch (v3) (4.41 KB, patch)
2012-03-27 11:30 PDT, Mark Capella [:capella]
dbolter: feedback+
Details | Diff | Splinter Review

Description David Bolter [:davidb] 2010-11-23 10:25:34 PST
Currently we expose as a paragraph.
Comment 1 David Bolter [:davidb] 2010-11-23 10:27:01 PST
Internally we'll map it to section.
Comment 2 Marco Zehe (:MarcoZ) 2011-11-01 05:53:40 PDT
Ping on this? What's the current spec requirement?
Comment 3 alexander :surkov 2011-11-01 06:17:46 PDT
(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"
Comment 4 David Bolter [:davidb] 2011-11-01 06:40:59 PDT
Some notes on <section> http://www.paciellogroup.com/blog/2011/03/html5-accessibility-chops-section-elements/
Comment 5 alexander :surkov 2012-02-08 04:13:25 PST
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.
Comment 6 David Bolter [:davidb] 2012-02-10 12:51:56 PST
I thought I had a local patch for this but don't see it. Marking GFB.
Comment 7 alexander :surkov 2012-02-12 19:57:28 PST
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
Comment 8 Michał Frontczak [:fxa90id] ♥ 2012-03-18 03:46:19 PDT
http://www.w3.org/TR/wai-aria/rdf_model ?
Comment 9 David Bolter [:davidb] 2012-03-19 12:59:13 PDT
(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 alexander :surkov 2012-03-19 18:22:13 PDT
(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?
Comment 11 alexander :surkov 2012-03-26 20:22:30 PDT
*** Bug 719852 has been marked as a duplicate of this bug. ***
Comment 12 Mark Capella [:capella] 2012-03-27 01:29:54 PDT
Created attachment 609654 [details] [diff] [review]
Patch (v1)

Requesting feedback from the mentor ...
Comment 13 alexander :surkov 2012-03-27 01:42:59 PDT
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
Comment 14 David Bolter [:davidb] 2012-03-27 06:23:45 PDT
(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.
Comment 15 David Bolter [:davidb] 2012-03-27 07:04:36 PDT
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?
Comment 16 Mark Capella [:capella] 2012-03-27 07:10:15 PDT
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 ...
Comment 17 Mark Capella [:capella] 2012-03-27 07:54:24 PDT
Created attachment 609709 [details] [diff] [review]
Patch (v2)

Changed as discussed ...
Comment 18 David Bolter [:davidb] 2012-03-27 10:27:27 PDT
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.
Comment 19 Mark Capella [:capella] 2012-03-27 11:30:00 PDT
Created attachment 609811 [details] [diff] [review]
Patch (v3)

Added reorg bug refererence.
Comment 20 David Bolter [:davidb] 2012-03-27 11:39:45 PDT
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 :)
Comment 22 Ed Morley [:emorley] 2012-03-28 14:00:26 PDT
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

Note You need to log in before you can comment on or make changes to this bug.