implement accessible objects for xforms messages

RESOLVED FIXED

Status

()

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

People

(Reporter: surkov, Assigned: surkov)

Tracking

({access})

Trunk
access
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 attachments, 5 obsolete attachments)

(Assignee)

Description

12 years ago
XForms has two types of messages: ephemeral and non ephemeral. Non ephemeral messages is just window opened by window.openDialog() method. Ephemeral messages behaves oneself like tooltips. Ephemeral messages implementation for xhtml is html:div, that div has absolute position and when message become to be visible then div changes the it's display style. Ephemeral messages implementation for xul is based on xul:tooltip (currently, still on xul:popup).

What events should I send when message changes state on visible/hidden? Do regular opened windows/xul:tooltip do it already?
(Assignee)

Updated

12 years ago
Assignee: aaronleventhal → surkov.alexander

Comment 1

12 years ago
(In reply to comment #0)
> XForms has two types of messages: ephemeral and non ephemeral. Non ephemeral
> messages is just window opened by window.openDialog() method. Ephemeral
> messages behaves oneself like tooltips. Ephemeral messages implementation for
> xhtml is html:div, that div has absolute position and when message become to be
> visible then div changes the it's display style. Ephemeral messages
> implementation for xul is based on xul:tooltip (currently, still on xul:popup).
> 
> What events should I send when message changes state on visible/hidden? Do
> regular opened windows/xul:tooltip do it already?
> 


We also have inline alerts which fit neither of these descriptions.  They are shown upon xforms-invalid and disappear upon xforms-valid, I believe.

Comment 2

12 years ago
Perfect. If those nodes can be marked as ROLE_ALERT they'll be spoken when they appear. Might be able to do that with xhtml:role="wairole:alert". Or you can create via nsIAccessibleProvider. An AlertActive event can be fired on the node, if it's not getting the EVENT_ALERT when it appears.

Comment 3

12 years ago
I don't think we do a goodgreat  handling XUL tooltips right now, but it's not so critical because we use the tooltiptext for the accessible description via GetDescription(). 

Let's create accessible objects for tooltips eventually, but when we do we need to handle all tooltips at the same time. There are DHTML tooltips as well.
(Assignee)

Comment 4

12 years ago
Created attachment 236537 [details] [diff] [review]
patch

The patch doesn't work since xforms message never become visible (excepting visible inline alert) and accessible objects can't be created for non visible elements. That's a problem.

XForms non ephemeral messages works corresponding following schema:
1) If xforms message have @src attribte then we open new window and load URL specified by @src attribute.
2) If xforms message is bound to instance node then we open new window with document that contains data of instance node.
3) If xforms message is inline then we open new window with that inline content.

XHTML XForms ephemeral messages shows html:div with message content in absolute position.
XUL XForms ephemeral messages shows popup with message content.

Comment 5

12 years ago
Test cases please :)
I want to see all these different types of XForms messages.
(Assignee)

Comment 6

12 years ago
Created attachment 237095 [details]
message testcase

(In reply to comment #5)
> Test cases please :)
> I want to see all these different types of XForms messages.
> 

Testcase shows three types of messages: modal, modeless and ephemeral. Aler, help and hint messages use one of these types.

Comment 7

12 years ago
I see now. I was expecting something like DHTML alerts:
http://www.mozilla.org/access/dhtml/alert
Is there anything like that?

The ephemeral messages (tooltips) can be exposed for now as the accessible descriptions.
The modal and modeless work pretty much already, although they should expose ROLE_ALERT instead of ROLE_DIALOG (not sure how it manages to get ROLE_DIALOG, but that's pretty good already). This is probably just a P3 in terms of priority.

Instead of sublcassing nsRootAccessible you would probably just change:
http://lxr.mozilla.org/seamonkey/source/accessible/src/base/nsRootAccessible.cpp#162
Or, where the dialog is created in XBL, you could put xhtml:role="wairole:alert" at the root element.

Comment 8

12 years ago
Comment on attachment 236537 [details] [diff] [review]
patch

After talking with Alexander on IRC, I understand that ephemeral msgs aren't tooltips, so I will look again.
Attachment #236537 - Flags: review?(aaronleventhal)
(Assignee)

Comment 9

12 years ago
Created attachment 237123 [details]
alert testcase

Updated

12 years ago
Keywords: access

Comment 10

12 years ago
Comment on attachment 236537 [details] [diff] [review]
patch

Don't use event_alert_high/medium/low because there is no equivalent on ATK, and they're not used by Windows ATs. In fact we shoehorn INVALID and REQUIRED into those states on Windows. In the future we may use this once we construct better APIs, but just remove it for now.

Don't fire any special events for the alerts that have their own window. The toolkits fire all the necessary events in that case.
Attachment #236537 - Flags: review?(aaronleventhal) → review-
(Assignee)

Comment 11

12 years ago
(In reply to comment #10)
> 
> Don't fire any special events for the alerts that have their own window. The
> toolkits fire all the necessary events in that case.
> 

But will be that messages readed automatically if they are just windows even if they have role="alert"?

Comment 12

12 years ago
They'll be read automatically if they're windows and have ROLE_ALERT. Those screen readers which don't do that have a bug.
(Assignee)

Comment 13

12 years ago
(In reply to comment #12)
> They'll be read automatically if they're windows and have ROLE_ALERT. Those
> screen readers which don't do that have a bug.
> 

I tried http://www.mozilla.org/access/dhtml/alert, when alert is show then EVENT_ALERT event is not fired. Should EVENT_ALERT be fired when element of alert role become to be visible?
Status: NEW → ASSIGNED
(Assignee)

Comment 14

12 years ago
(In reply to comment #13)

> I tried http://www.mozilla.org/access/dhtml/alert, when alert is show then
> EVENT_ALERT event is not fired. Should EVENT_ALERT be fired when element of
> alert role become to be visible?
> 

The first reason is nsDocAccessible::mIsContentLoaded == PR_FALSE (see http://lxr.mozilla.org/mozilla/source/accessible/src/base/nsDocAccessible.cpp#1250) if page is loaded into browser before than ally code is activated. mIsContentLoaded is changed in two places (nsDocAccessible::nsDocAccessible() http://lxr.mozilla.org/mozilla/source/accessible/src/base/nsDocAccessible.cpp#116 and nsDocAccessible::FireDocLoadEvents http://lxr.mozilla.org/mozilla/source/accessible/src/base/nsDocAccessible.cpp#682). Right now I have not idea how to fix it.
(Assignee)

Comment 15

12 years ago
(In reply to comment #13)

> I tried http://www.mozilla.org/access/dhtml/alert, when alert is show then
> EVENT_ALERT event is not fired. Should EVENT_ALERT be fired when element of
> alert role become to be visible?
> 

The second reason is nsDocAccessible::InvalidateCacheSubtree() isn't called when I show alert by modifying display/visibility styles.

Comment 16

12 years ago
Yeah, do a reload and then try. The mIsContentLoaded gets set after the first load where a11y is initialized. This is not normally a problem since the AT is usually loaded before Firefox comes up, but it should be fixed.

Also, do you have an updated tree with the fix from bug 354745?
(Assignee)

Comment 17

12 years ago
(In reply to comment #16)

> Also, do you have an updated tree with the fix from bug 354745?
> 

Yes, nsIAccessibilityService::InvalidateSubtreeFor() method isn't called does layout code should call it when display/visibility are changed?

Comment 18

12 years ago
> Yes, nsIAccessibilityService::InvalidateSubtreeFor() method isn't called does
> layout code should call it when display/visibility are changed?

Yes, if you search for that method you'll see it's used in presshell, framemanager and cssframeconstructor.
(Assignee)

Comment 19

12 years ago
Created attachment 243118 [details] [diff] [review]
patch2

The patch uses xhtml2:role approach. I skiped support of ephemeral messages in xul since they don't longer work (bug 357604).
Attachment #236537 - Attachment is obsolete: true
Attachment #243118 - Flags: review?(aaronleventhal)

Comment 20

12 years ago
Comment on attachment 243118 [details] [diff] [review]
patch2

The role attribute is not part of the XUL or SVG namespace. So why the change to GetRoleAttribute()?

Don't use xhtml2 anymore for the role attribute, use the xhtml1.x namespace. It's been moved there.

What does the change to loadMessageURL do? Is there more of a core fix required there?

Most of this looks like something that Aaron Reed needs to review. The only mozilla/accessible change is something that looks to me like it should go away.
Attachment #243118 - Flags: review?(aaronleventhal) → review-
(Assignee)

Comment 21

12 years ago
Comment on attachment 243118 [details] [diff] [review]
patch2

(In reply to comment #20)
> (From update of attachment 243118 [details] [diff] [review] [edit])
> The role attribute is not part of the XUL or SVG namespace. So why the change
> to GetRoleAttribute()?

I like to use dhtml roles for custom namespace elements like for XUL. Should XUL have special mechanism to provide role support?

> What does the change to loadMessageURL do? Is there more of a core fix required
> there?

Previously I used xul:browser that was used loadMessageURL, but I failed to use xhtml:role with them therefore I replaced it on xul:iframe.

I didn't change ns though, I will do it after your comments.
Attachment #243118 - Flags: review?(aaronr)

Comment 22

12 years ago
> I like to use dhtml roles for custom namespace elements like for XUL. Should
> XUL have special mechanism to provide role support?

This should be enough:


     ...
     <xul:foo 
      xmlns:xhtml=""http://www.w3.org/1999/xhtml"
      xmlns:wairole="http://www.w3.org/2005/01/wai-rdf/GUIRoleTaxonomy#"
      xhtml:role="wairole:bar">
(Assignee)

Comment 23

12 years ago
Created attachment 243474 [details] [diff] [review]
patch3
Attachment #243118 - Attachment is obsolete: true
Attachment #243474 - Flags: review?(aaronr)
Attachment #243118 - Flags: review?(aaronr)

Comment 24

12 years ago
Created attachment 243734 [details]
testcase - inline alert

another testcase to try.  Please verify that this works.

Comment 25

12 years ago
Comment on attachment 243474 [details] [diff] [review]
patch3

I don't have a problem with the code.  But what about hints and inline alerts?  I don't see how they are handled here.  Don't they need some kind of role on their div's, too?  Or is that a followup bug?

Comment 26

12 years ago
Comment on attachment 243474 [details] [diff] [review]
patch3

removing from my review queue.  Please re-request if you have a new patch or decide to open a new bug (or if I am just wrong and they are handled and I just don't see it :-)
Attachment #243474 - Flags: review?(aaronr)
(Assignee)

Comment 27

12 years ago
(In reply to comment #25)
> (From update of attachment 243474 [details] [diff] [review] [edit])
> I don't have a problem with the code.  But what about hints and inline alerts? 
> I don't see how they are handled here.  Don't they need some kind of role on
> their div's, too?  Or is that a followup bug?
> 

ALERT_EVENT shouldn't be fired for xforms hints since they are mapped to description property of accessible object. But I'm not sure about inline alerts. AaronLev how we should handle inline alerts (please look at AaronR's testcase)?

Comment 28

12 years ago
Inline alert is almost exactly what we want to fire EVENT_ALERT for. It needs to be fired so the alert text is spoken when it is dynamically made visible.

Comment 29

12 years ago
So, the short answer is "Yes. Fire EVENT_ALERT for inline alerts"

Comment 30

12 years ago
BTW, if the inline alert has ROLE_ALERT, the accessibility magic should work to fire EVENT_ALERT automatically.
(Assignee)

Comment 31

12 years ago
Created attachment 243938 [details] [diff] [review]
patch4

inline alerts supported both for xul and xhtml
Attachment #243474 - Attachment is obsolete: true
Attachment #243938 - Flags: review?(aaronr)

Comment 32

12 years ago
Comment on attachment 243938 [details] [diff] [review]
patch4

>Index: extensions/xforms/resources/content/xforms-xhtml.xml
>===================================================================
>RCS file: /cvsroot/mozilla/extensions/xforms/resources/content/xforms-xhtml.xml,v
>retrieving revision 1.15
>diff -u -8 -p -r1.15 xforms-xhtml.xml
>--- extensions/xforms/resources/content/xforms-xhtml.xml	21 Sep 2006 16:49:13 -0000	1.15
>+++ extensions/xforms/resources/content/xforms-xhtml.xml	29 Oct 2006 00:54:00 -0000
>@@ -49,17 +49,18 @@
> <!ENTITY % xformsDTD SYSTEM "chrome://xforms/locale/xforms.dtd">
>   %xformsDTD;
> ]>
> 
> <bindings id="xformsBindingsForXHTML"
>           xmlns="http://www.mozilla.org/xbl"
>           xmlns:html="http://www.w3.org/1999/xhtml"
>           xmlns:xbl="http://www.mozilla.org/xbl"
>-          xmlns:mozType="http://www.mozilla.org/projects/xforms/2005/type">
>+          xmlns:mozType="http://www.mozilla.org/projects/xforms/2005/type"
>+          xmlns:wairole="http://www.w3.org/2005/01/wai-rdf/GUIRoleTaxonomy#">
> 
> 
>   <!-- OUTPUT: <DEFAULT> -->
>   <binding id="xformswidget-output"
>            extends="chrome://xforms/content/xforms.xml#xformswidget-output-base">
>     <content>
>       <children includes="label"/>
>       <!-- XXX initialize span with a space until repeat is xbl-ized.  Part
>@@ -427,33 +428,35 @@
>       </method>
>     </implementation>
>   </binding>
> 
> 
>   <!-- ALERT: <DEFAULT> -->
>   <binding id="xformswidget-alert"
>            extends="chrome://xforms/content/xforms.xml#xformswidget-alert-base">
>-    <content>
>+    <content html:role="wairole:alert">
>       <html:div anonid="container" class="-moz-xforms-message-container">

Are you sure that this will work?  I'd think that you'd want the role to be on the html:div, not on the xbl:content since the div is actually in the anonymous content.


>@@ -462,54 +465,54 @@
>             }
>           };
>         </body>
>       </method>
>     </implementation>
>   </binding>
> 
> 
>-  <!-- MESSAGE: EPHEMERAL, HINT: <DEFAULT>
>-    The widget reuses content of xformswidget-alert.
>-  -->
>-  <binding id="xformswidget-ephemeral-message"
>+    <!-- HINT: <DEFAULT> -->
>+  <binding id="xformswidget-hint"
>            extends="#xformswidget-alert">

nit: you are indenting the comment.  Other places in this file we don't, so we probably shouldn't here.  It should start in the same column as the binding element for which it is a comment.

>-
>-    <implementation implements="nsIXFormsEphemeralMessageUI">
>+     <implementation implements="nsIXFormsEphemeralMessageUI">

nit: you shouldn't add another indent here.  We should keep the indent at 2 spaces all throughout the file.

>Index: extensions/xforms/resources/content/xforms-xul.xml
>===================================================================
>RCS file: /cvsroot/mozilla/extensions/xforms/resources/content/xforms-xul.xml,v
>retrieving revision 1.9
>diff -u -8 -p -r1.9 xforms-xul.xml
>--- extensions/xforms/resources/content/xforms-xul.xml	30 Aug 2006 15:01:43 -0000	1.9
>+++ extensions/xforms/resources/content/xforms-xul.xml	29 Oct 2006 00:54:00 -0000
>@@ -350,17 +350,20 @@
>       </method>
>     </implementation>
>   </binding>
> 
> 
>   <!-- ALERT: <DEFAULT> -->
>   <binding id="xformswidget-alert"
>            extends="chrome://xforms/content/xforms.xml#xformswidget-alert-base">
>-    <content>
>+
>+    <content xmlns:xhtml="http://www.w3.org/1999/xhtml"
>+             xmlns:wairole="http://www.w3.org/2005/01/wai-rdf/GUIRoleTaxonomy#"
>+             xhtml:role="wairole:alert">
>       <xul:deck anonid="contentswitcher" flex="1" selectedIndex="1">

same here...is it ok to have the xhtml:role attribute on the xbl:content or would it be better to have it on the xul:deck?

r-'ing just to make sure that the xhtml:role is supposed to go where this patch has it or not.  Otherwise just nits.
Attachment #243938 - Flags: review?(aaronr) → review-
(Assignee)

Comment 33

12 years ago
Created attachment 244160 [details] [diff] [review]
patch5

(In reply to comment #32)

> >+    <content html:role="wairole:alert">
> >       <html:div anonid="container" class="-moz-xforms-message-container">
> 
> Are you sure that this will work?  I'd think that you'd want the role to be on
> the html:div, not on the xbl:content since the div is actually in the anonymous
> content.

> >+    <content xmlns:xhtml="http://www.w3.org/1999/xhtml"
> >+             xmlns:wairole="http://www.w3.org/2005/01/wai-rdf/GUIRoleTaxonomy#"
> >+             xhtml:role="wairole:alert">
> >       <xul:deck anonid="contentswitcher" flex="1" selectedIndex="1">
> 
> same here...is it ok to have the xhtml:role attribute on the xbl:content or
> would it be better to have it on the xul:deck?

@role attributes on content is used only for inline alerts. When alert becomes visible then accessibility code will not look inside of element that is visibile. So I should set @role attribute on xforms:alert directly.

I changed only nits :)
Attachment #243938 - Attachment is obsolete: true
Attachment #244160 - Flags: review?(aaronr)

Comment 34

12 years ago
But have you tested to ensure it fires EVENT_ALERT?

If we can use XBL to set attributes on a the original non-anonymous element I'd be really surprised.
(Assignee)

Comment 35

12 years ago
(In reply to comment #34)
> But have you tested to ensure it fires EVENT_ALERT?

Yes. It fires.

> If we can use XBL to set attributes on a the original non-anonymous element I'd
> be really surprised.
> 

That's XBL can do. Fine feature. Also the feature is used by toolkit too (see for example, http://lxr.mozilla.org/mozilla/source/toolkit/content/widgets/menulist.xml#17)

Comment 36

12 years ago
Wow, so will the attribute be present in the normal DOM, or do we have a concept of anonymous attributes on non-anonymous elements?

Anyway, that's great. Really great, because it means we can use the accessibility role and property attributes more easily for creating new widgets.

Comment 37

12 years ago
(In reply to comment #36)
> Wow, so will the attribute be present in the normal DOM, or do we have a
> concept of anonymous attributes on non-anonymous elements?
> 

No, we don't have concept of anonymous attributes, but that sounds interesting :)
(Assignee)

Comment 38

12 years ago
(In reply to comment #36)
> Wow, so will the attribute be present in the normal DOM, or do we have a
> concept of anonymous attributes on non-anonymous elements?

attribute is present in the normal DOM.

> Anyway, that's great. Really great, because it means we can use the
> accessibility role and property attributes more easily for creating new
> widgets.
> 

Agree, very nice feature :)
(Assignee)

Comment 39

12 years ago
Comment on attachment 244160 [details] [diff] [review]
patch5

So, if aaron had only nits, then I guess I can ask you to review don't wating aaron's one.
Attachment #244160 - Flags: review?(Olli.Pettay)

Comment 40

12 years ago
Comment on attachment 244160 [details] [diff] [review]
patch5

>Index: extensions/xforms/resources/content/xforms-message.xul
>===================================================================
>RCS file: /cvsroot/mozilla/extensions/xforms/resources/content/xforms-message.xul,v
>retrieving revision 1.2
>diff -u -8 -p -r1.2 xforms-message.xul
>--- extensions/xforms/resources/content/xforms-message.xul	19 Sep 2006 07:54:58 -0000	1.2
>+++ extensions/xforms/resources/content/xforms-message.xul	31 Oct 2006 02:30:29 -0000
>@@ -35,27 +35,35 @@
>    - the terms of any one of the MPL, the GPL or the LGPL.
>    -
>    - ***** END LICENSE BLOCK ***** -->
> 
> <?xml-stylesheet href="chrome://global/skin/" type="text/css"?>
> 
> <dialog id="XFormsMessageDialog"
>         xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"
>+        xmlns:xul="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"

No need for this.

> 
>-  <browser flex="1" id="browser"/>
>+  <xul:iframe xmlns:xhtml="http://www.w3.org/1999/xhtml"
>+              xmlns:wairole="http://www.w3.org/2005/01/wai-rdf/GUIRoleTaxonomy#"
>+              id="frame" flex="1"
>+              style="display: none;"
>+              xhtml:role="wairole:alert"/>


<iframe .... should be enough.
Attachment #244160 - Flags: review?(Olli.Pettay) → review+
(Assignee)

Comment 41

12 years ago
Created attachment 244195 [details] [diff] [review]
patch6
Attachment #244160 - Attachment is obsolete: true
Attachment #244195 - Flags: review?(aaronr)
Attachment #244160 - Flags: review?(aaronr)

Comment 42

12 years ago
Comment on attachment 244195 [details] [diff] [review]
patch6

Of course all of this assumes that the user didn't have a html:role on the control to begin with.

We need answer from accessibility guys whether we can blindly overwrite html:role like this.
Attachment #244195 - Flags: review?(aaronr) → review+

Comment 43

12 years ago
Already checked in by aaronr.
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.