614310 - Map <section> to pane (like role="region")

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[David Bolter [:davidb] (NeedInfo me for attention)]

Currently we expose as a paragraph.

Comment 1

[David Bolter [:davidb] (NeedInfo me for attention)]

Internally we'll map it to section.

Comment 2

[Marco Zehe (:MarcoZ)]

Ping on this? What's the current spec requirement?

Comment 3

[alexander :surkov (:asurkov)]

(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] (NeedInfo me for attention)]

Some notes on <section> http://www.paciellogroup.com/blog/2011/03/html5-accessibility-chops-section-elements/

Comment 5

[alexander :surkov (:asurkov)]

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] (NeedInfo me for attention)]

I thought I had a local patch for this but don't see it. Marking GFB.

Comment 7

[alexander :surkov (:asurkov)]

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] ♥]

http://www.w3.org/TR/wai-aria/rdf_model ?

Comment 9

[David Bolter [:davidb] (NeedInfo me for attention)]

(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 (:asurkov)]

(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 12

[Mark Capella [:capella]]

Attachment: Patch (v1)

Requesting feedback from the mentor ...

Comment 13

[alexander :surkov (:asurkov)]

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] (NeedInfo me for attention)]

(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] (NeedInfo me for attention)]

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]]

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]]

Attachment: Patch (v2)

Changed as discussed ...

Comment 18

[David Bolter [:davidb] (NeedInfo me for attention)]

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]]

Attachment: Patch (v3)

Added reorg bug refererence.

Comment 20

[David Bolter [:davidb] (NeedInfo me for attention)]

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 21

[alexander :surkov (:asurkov)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/3083df9ca118

Comment 22

[Ed Morley [:emorley]]

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