The default bug view has changed. See this FAQ.

Map ARIA role "form".

RESOLVED FIXED in mozilla14

Status

()

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

People

(Reporter: davidb, Assigned: capella)

Tracking

(Blocks: 1 bug)

unspecified
mozilla14
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [good first bug][mentor=eitan@monotonous.org][lang=c++][ARIA conformance])

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

5 years ago
For some reason this one is missing from our nsARIAMap.
(Reporter)

Comment 1

5 years ago
http://www.w3.org/WAI/PF/aria/roles#form
Blocks: 343213
(Reporter)

Updated

5 years ago
Whiteboard: [ARIA conformance]

Comment 2

5 years ago
spec: http://www.w3.org/WAI/PF/aria/roles#form
aria impl guide: http://www.w3.org/WAI/PF/aria-implementation/#mapping_role_table

add form entry to nsARIAMap.cpp

add a test to test_aria_roles.html and elm/test_landmarks.html
Whiteboard: [ARIA conformance] → [good first bug][mentor=eitan@monotonous.org][lang=c++][ARIA conformance]
(Assignee)

Comment 3

5 years ago
Takes it ... could have rolled it in with # 737156 ... separate works also
Assignee: nobody → markcapella
Status: NEW → ASSIGNED
(Assignee)

Updated

5 years ago
Depends on: 737156
(Assignee)

Comment 4

5 years ago
Created attachment 608550 [details] [diff] [review]
Patch (v1)

First try ... up for review for correctness / completeness ...
Attachment #608550 - Flags: feedback?(eitan)
Comment on attachment 608550 [details] [diff] [review]
Patch (v1)

This looks correct, thank you!
Attachment #608550 - Flags: feedback?(eitan) → feedback+
(Assignee)

Comment 6

5 years ago
Created attachment 608962 [details] [diff] [review]
Patch (v2)

Added :surkov as review to commit message, re-posted
Attachment #608962 - Flags: review?(surkov.alexander)

Updated

5 years ago
Attachment #608550 - Attachment is obsolete: true

Comment 7

5 years ago
Comment on attachment 608962 [details] [diff] [review]
Patch (v2)

Review of attachment 608962 [details] [diff] [review]:
-----------------------------------------------------------------

::: accessible/tests/mochitest/elm/test_landmarks.html
@@ -28,1 @@
>        testRole("main", ROLE_DOCUMENT);

don't remove empty line, it's used to separate HTML landmarks and ARIA landmarks

@@ +40,5 @@
>        testAttrs("footer", {"tag" : "FOOTER"}, true);
>        testAttrs("article", {"tag" : "ARTICLE"}, true);
>        testAttrs("aside", {"tag" : "ASIDE"}, true);
>        testAttrs("main", {"tag" : "ARTICLE"}, true); // no override expected
> +      testAttrs("form", {"tag" : "ARTICLE"}, true); // no override expected

no idea why that 'no override expected' mean, 'tag' is never gets overridden. pls remove those comments

@@ +74,1 @@
>  

pls refer to a bug number

::: accessible/tests/mochitest/test_aria_roles.html
@@ +34,5 @@
>        testRole("marquee_h1", ROLE_ANIMATION);
>  
>        // strong landmark
>        testRole("application", ROLE_APPLICATION);
> +      testRole("form", ROLE_FORM);

ok, clean dupe of test_landmarks.html but it makes sense to keep them both until we merge these two tests.

@@ -116,5 @@
>    <div role="search" id="search">search</div>
>  
> -  <!-- landmarks are tables -->
> -  <table role="application" id="application_table">application table
> -    <tr><td>hi<td></tr></table>

Bug 469688 - <table role="*"> should expose inner table structure only for weak roles; r=MarcoZ,surkov 

I think David missed the test for this case. I'd be nice if you can add it (in either case pls don't remove it).
Attachment #608962 - Flags: review?(surkov.alexander) → review+
(Assignee)

Comment 8

5 years ago
Created attachment 609279 [details] [diff] [review]
Patch (v3)

Nits addressed - re-requesting review
Attachment #608962 - Attachment is obsolete: true
Attachment #609279 - Flags: review?(surkov.alexander)

Comment 9

5 years ago
Comment on attachment 609279 [details] [diff] [review]
Patch (v3)

Review of attachment 609279 [details] [diff] [review]:
-----------------------------------------------------------------

since you rolled back the change <table role="application" id="application_table">application table instead of adding a test for it then please file bug for this

::: accessible/tests/mochitest/test_aria_roles.html
@@ +93,5 @@
>    <a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=469688">Mozilla Bug 469688</a>
>    <a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=469688">Mozilla Bug 520188</a>
>    <a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=529289">Mozilla Bug 529289</a>
>    <a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=529289">Mozilla Bug 607219</a>
> +  <a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=735645">Mozilla Bug 735645</a>

please put @title attribute on it
Attachment #609279 - Flags: review?(surkov.alexander) → review+

Comment 10

5 years ago
(In reply to alexander :surkov from comment #9)

> > +  <a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=735645">Mozilla Bug 735645</a>
> 
> please put @title attribute on it

btw, I'll do this locally before landing
(Assignee)

Comment 11

5 years ago
I wondered about it when copied the line above it ... will be sure to from now on ...

Comment 12

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/2302e6bc12d7
Target Milestone: --- → mozilla14
https://hg.mozilla.org/mozilla-central/rev/2302e6bc12d7
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED

Updated

5 years ago
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.