Closed Bug 398910 Opened 13 years ago Closed 13 years ago

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

Categories

(Core :: Disability Access APIs, defect)

defect
Not set
normal

Tracking

()

VERIFIED FIXED

People

(Reporter: aaronlev, Assigned: aaronlev)

References

Details

(Keywords: access)

Attachments

(2 files, 2 obsolete files)

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.
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.
Sure I don't mind, although in XUL these extra accessibility attributes are actually redundant with those already in XUL.
Some of them are, true. But it would take extra code to remove support for them.
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/474F3B57.80000@w3.org
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.

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

Blocks: 407401
FWIW this is all just code removal and comment fixes.
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
Attachment #292152 - Flags: review?(david.bolter) → review+
Attachment #292152 - Flags: review?(surkov.alexander)
Patch have support of activedescedant from bug 403260 (Lines 2302-2311    nsAccessible::GetFinalState(PRUint32 *aS ). Please remove it.
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'
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.
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"/>
- <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 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
Attachment #292152 - Flags: review?(surkov.alexander) → review+
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.
Attachment #292152 - Flags: superreview?(neil)
(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 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?
Attachment #292152 - Flags: superreview?(neil) → superreview+
Seeking
r+=neil for suite and toolkit
r+=mano for browser
r+=bienvenu for mail/mailnews changes
r+=aaronr for xforms
Attachment #292152 - Flags: review?(neil)
Attachment #292152 - Flags: review?(mano)
Attachment #292152 - Flags: review?(bienvenu)
Attachment #292152 - Flags: review?(aaronr)
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.
Attachment #292152 - Flags: review?(bienvenu) → review+
(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.
Attachment #292152 - Flags: review?(aaronr)
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.
Attachment #292152 - Flags: review?(neil) → review+
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?
Mano, Surkov caught that one in comment 15. I will fix before checking in.
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/.
Attachment #292152 - Flags: review?(mano) → review+
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.
Attachment #292152 - Flags: approval1.9?
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
Attachment #292152 - Flags: approval1.9? → approval1.9+
Attachment #292530 - Attachment is obsolete: true
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
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.
Verified using Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9b3) Gecko/2008020514 Firefox/3.0b3
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.