398910 - Non-namespaced ARIA role and property support in SVG, and possibly XUL

Status: VERIFIED FIXED

(Core :: Disability Access APIs, defect)

Description

[Aaron Leventhal]

http://simon.html5.org/specs/aria-proposal

In the spec SVG now allows role and aria-foo properties directly. SVG team wants to be consistent with XHTML. If we want to be even more consistent we should just allow it in XUL as well.

A decision will be made Thursday.

Comment 1

[Aaron Leventhal]

CC'ing Neil Rashbrook and Neil Deakin to see if they have anything against doing this for XUL as well. If we do it for XUL that would mean that everywhere we support ARIA, we can do it the same way, without namespaces.

Comment 2

[Neil Deakin]

Sure I don't mind, although in XUL these extra accessibility attributes are actually redundant with those already in XUL.

Comment 3

[Aaron Leventhal]

Some of them are, true. But it would take extra code to remove support for them.

Comment 4

[Aaron Leventhal]

At least role has been resolved.

See the RESOLUTION for SVG:

http://www.w3.org/2007/10/09-svg-minutes.html#item07

and for XHTML 2:

http://www.w3.org/mid/[e-mail]

Comment 5

[Aaron Leventhal]

Another resolution for ARIA hyphen:

In PFWG:

http://www.w3.org/2007/12/05-pf-minutes.html

As far as SVG, Doug Schepers and Chris Lilley say it's not their favorite solution, but they are willing to accept it in the interests of moving on to new topics.

Comment 6

[Doug Schepers]

(In reply to comment #5)
> 
> As far as SVG, Doug Schepers and Chris Lilley say it's not their favorite
> solution, but they are willing to accept it in the interests of moving on to
> new topics.

Yes, SVG would likely have to support both "aria-*" and the namespaced version, and possibly set up some sort of equivalency; further, it's more work for both SVG and WAI, and requires updated schemae for each language ARIA is meant to work with for each update to ARIA, but it's doable.  

I've tested this (e.g. <circle ... role="foo" aria-bar="blah" />) in all SVG 1.1 UAs I have handy, and it doesn't cause any problems.  All the values are exposed in the DOM and in the subset of CSS that UA supported.  

There would not be any mandate in the SVG Tiny 1.2 spec (it's too late for that), but we intend to include this functionality in a later version; we see no reason to delay implementing this now.

FWIW, it's not that we just want to move on to other issues, but rather that we want this functionality implemented.

Comment 7

[Aaron Leventhal]

Attachment: WIP -- need to fix atoms in nsARIAMap -- they are being initialized as nsnull because they aren't allocated yet

Comment 8

[Aaron Leventhal]

Attachment: Works, but needs testing with a suite of examples compliant with the new markup -- Dojo & John Gunderson's examples would be a good start

Comment 9

[Aaron Leventhal]

FWIW this is all just code removal and comment fixes.

Comment 10

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

Comment on attachment 292152 [details] [diff] [review]
Works, but needs testing with a suite of examples compliant with the new markup -- Dojo & John Gunderson's examples would be a good start

Wow I understand the tedious aspect of this patch ;)

Overall the patch looks good to me. I lxr'ed the codebase for wairole and don't see anything you've missed.

You've added a few ifdef blocks for some debugging that probably relates to a separate bug?

>+#ifdef DEBUG_CARET
>+          // Test caret line # -- fire an EVENT_ALERT on the focused node so we can watch the
>+          // line-number object attribute on it
>+          nsCOMPtr<nsIAccessible> accForFocus;
>+          GetAccService()->GetAccessibleFor(gLastFocusedNode, getter_AddRefs(accForFocus));
>+          nsAccUtils::FireAccEvent(nsIAccessibleEvent::EVENT_ALERT, accForFocus);
>+#endif


>+#ifdef DEBUG
>+  // Capture mouse over events and fire fake DRAGDROPSTART event to simplify
>+  // debugging a11y objects with event viewers
>+  "mouseover",
>+#endif

>+#ifdef DEBUG
>+  else if (eventType.EqualsLiteral("mouseover")) {
>+    nsAccUtils::FireAccEvent(nsIAccessibleEvent::EVENT_DRAGDROP_START, accessible);
>+  }
>+#endif

Comment 11

[alexander :surkov (:asurkov)]

Patch have support of activedescedant from bug 403260 (Lines 2302-2311    nsAccessible::GetFinalState(PRUint32 *aS ). Please remove it.

Comment 12

[alexander :surkov (:asurkov)]

I would like to move up the following comment:

// The multiselectable and other waistate attributes only take affect
// when dynamic content role is present, we already check that abbove

looks it makes sense to keep before
  if (aAttribute == nsAccessibilityAtoms::aria_multiselectable &&
      aContent->HasAttr(kNameSpaceID_None, nsAccessibilityAtoms::role)) {

nit: misspelling in 'abbove'

Comment 13

[alexander :surkov (:asurkov)]

Along with new DEBUG section as noticed David you introduced additional events mapping:

EVENT_SYSTEM_DRAGDROPSTART nsIAccessibleEvent::EVENT_DRAGDROP_START

It doesn't sound it's related with this bug. Please remove it.

Comment 14

[alexander :surkov (:asurkov)]

nit: while you are here you can replace attributes indentation for
<vbox id="dataSources" style="overflow: auto; -moz-appearance: listbox" align="left" flex="1" role="group"/>

on 

<vbox id="dataSources"
      style="overflow: auto; -moz-appearance: listbox"
      align="left" flex="1"
      role="group"/>

Comment 15

[alexander :surkov (:asurkov)]

- <html:input class="datetimepicker-input textbox-input" anonid="input-one"
+ <input class="datetimepicker-input textbox-input" anonid="input-one"

You put html:input into xbl namespace.

Comment 16

[alexander :surkov (:asurkov)]

Comment on attachment 292152 [details] [diff] [review]
Works, but needs testing with a suite of examples compliant with the new markup -- Dojo & John Gunderson's examples would be a good start

r=me with fixed

Comment 17

[Aaron Leventhal]

Comment on attachment 292152 [details] [diff] [review]
Works, but needs testing with a suite of examples compliant with the new markup -- Dojo & John Gunderson's examples would be a good start

Thanks, especially for noticing the html:input issue.

Surkov, the DRAGDROPSTART stuff is very useful code for debugging. It allows you to hover over something and see it's properties in the event monitor. I think we should keep it in.

Comment 18

[alexander :surkov (:asurkov)]

(In reply to comment #17)
> (From update of attachment 292152 [details] [diff] [review])
> Thanks, especially for noticing the html:input issue.
> 
> Surkov, the DRAGDROPSTART stuff is very useful code for debugging. It allows
> you to hover over something and see it's properties in the event monitor. I
> think we should keep it in.
> 

Ok, it's up to you. Just sometimes it's hard to follow similar patches through cvs history.

Comment 19

[[e-mail]]

Comment on attachment 292152 [details] [diff] [review]
Works, but needs testing with a suite of examples compliant with the new markup -- Dojo & John Gunderson's examples would be a good start

>Index: accessible/src/base/nsARIAPropertyList.h
>===================================================================
>RCS file: accessible/src/base/nsARIAPropertyList.h
>diff -N accessible/src/base/nsARIAPropertyList.h
>--- accessible/src/base/nsARIAPropertyList.h	25 Sep 2007 01:19:03 -0000	1.1
>+++ /dev/null	1 Jan 1970 00:00:00 -0000
Poor little thing, dead already, didn't even get to see Christmas...sob...

> #define kNameSpaceID_XMLEvents 11
>-#define kNameSpaceID_XHTML2_Unofficial    12
>-#define kNameSpaceID_WAIRoles  13
>-#define kNameSpaceID_WAIProperties 14
>-#define kNameSpaceID_LastBuiltin          14 // last 'built-in' namespace
>+#define kNameSpaceID_LastBuiltin          12 // last 'built-in' namespace
This looks wrong to me, you removed three name space IDs leaving 11.
sr=me with this fixed.

Do you need a content peer to sign off on the changes to the relavent fiels?

Comment 20

[Aaron Leventhal]

Seeking
r+=neil for suite and toolkit
r+=mano for browser
r+=bienvenu for mail/mailnews changes
r+=aaronr for xforms

Comment 21

[David :Bienvenu]

Comment on attachment 292152 [details] [diff] [review]
Works, but needs testing with a suite of examples compliant with the new markup -- Dojo & John Gunderson's examples would be a good start

r=bienvenu for the mail parts.

Comment 22

[alexander :surkov (:asurkov)]

(In reply to comment #20)
> Seeking
> r+=aaronr for xforms
> 

I granted r+ for xforms part. Though usually xforms requires two r+ but possibly not for this case.

Comment 23

[[e-mail]]

Comment on attachment 292152 [details] [diff] [review]
Works, but needs testing with a suite of examples compliant with the new markup -- Dojo & John Gunderson's examples would be a good start

r=me for suite.

Comment 24

[Asaf Romano (gone)]

How is this change correct?
-          <html:input class="datetimepicker-input textbox-input" anonid="input-ampm"
+          <input class="datetimepicker-input textbox-input" anonid="input-ampm"
                       size="2" maxlength="2" flex="1" chromedir="&locale.dir;"
                       xbl:inherits="disabled,readonly"/>

input isn't part of the xbl namespace, is it?

Comment 25

[Aaron Leventhal]

Mano, Surkov caught that one in comment 15. I will fix before checking in.

Comment 26

[Asaf Romano (gone)]

Comment on attachment 292152 [details] [diff] [review]
Works, but needs testing with a suite of examples compliant with the new markup -- Dojo & John Gunderson's examples would be a good start

besides that, r=mano for the changes in toolkit/ and browser/.

Comment 27

[Aaron Leventhal]

Comment on attachment 292152 [details] [diff] [review]
Works, but needs testing with a suite of examples compliant with the new markup -- Dojo & John Gunderson's examples would be a good start

Seeking a= to prevent authors from using the incorrect markup moving forward.

Comment 28

[Mike Connor [:mconnor]]

Comment on attachment 292152 [details] [diff] [review]
Works, but needs testing with a suite of examples compliant with the new markup -- Dojo & John Gunderson's examples would be a good start

a=mconnor on behalf of drivers for checkin, please make sure we get test coverage on this ASAP

Comment 29

[Aaron Leventhal]

Attachment: Address all comments -- ready for more testing

Comment 30

[Aaron Leventhal]

Attachment: Address all comments -- ready for more testing

Comment 31

[Aaron Leventhal]

We created a test build and tested all of the Dojo widgets, with screen readers on Windows and Linux. No defects related to this patch were found.

Comment 32

[Marco Zehe (:MarcoZ)]

Verified using Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9b3) Gecko/2008020514 Firefox/3.0b3