Open Bug 448235 Opened 16 years ago Updated 2 years ago

Thunderbird replied and forwarded states not exposed via AT-SPI

Categories

(Core :: Disability Access APIs, defect)

x86
Linux
defect

Tracking

()

People

(Reporter: jdiggs, Unassigned)

References

(Blocks 1 open bug)

Details

(Keywords: access, Whiteboard: [bk1])

Attachments

(1 file)

In the Thunderbird message list, messages that the user has replied to or forwarded are indicated by a graphic/icon at the start of the message's subject. The presence of this graphic/icon is not being exposed via AT-SPI.
It definitely won't work on windows, probably until we'll fix the bug 367905.

So that's how they put those images:

treechildren::-moz-tree-image(subjectCol, replied) {
  list-style-image: url("chrome://messenger/skin/icons/symbol-replied.png") !important;
}

treechildren::-moz-tree-image(subjectCol, forwarded) {
  list-style-image: url("chrome://messenger/skin/icons/symbol-forwarded.png") !important;
}

Now I haven't any idea how to fix our code for accessible tree to support this specific case.
The only one idea I have is to extend nsITreeView interface (nsITreeView2 probably) by the method that should return localized properties that are important. Some kind of:

interface nsITreeView2 : public nsITreeView
{
  void getSemanticCellProperties(in long row, in nsITreeColumn col, in nsISupportsArray properties);
}

this method will return an array of localized strings.

So in our case if we have forwarded and replied letter with the subject "Bla bla bla" then accessible name will be "forwarded replied Bla bla bla".

How does it sound?
cc'ed paul
nsIAccessibleTreeView sounds as a better name.
the "property" keyword is not a good idea since it refers to style stuff.

What about:
getSemanticCellStrings(in long row, in nsITreeColumn col, in nsISupportsArray strings);
(In reply to comment #4)
> nsIAccessibleTreeView sounds as a better name.
> the "property" keyword is not a good idea since it refers to style stuff.

I'm not sure I would like to have special interfaces for this and any more special interface for accessibility  hosted in content.

> What about:
> getSemanticCellStrings(in long row, in nsITreeColumn col, in nsISupportsArray
> strings);
> 

I'm not sure about "strings" because it sounds too general. Though I'm agree "properties" doesn't sound totally right. I would used something like "localized properties" but it sounds too long.
One question: On Windows, a single entry for me in the Thunderbird message list looks like this:
      Re: problems with sound in intrepid    Francesco Fumanti  mike  0  10:27  10:27  New  3KB            49055    

Notice the "new" before the size?

Another message looks like this:

      [Bug 424195] Start running a11y Mochitests regularly.    bugzilla-daemon@mozilla.org  marco.zehe@googlemail.com  0  04:14  04:14  Read  2KB            48990    

Notice the "Read"?

If you can find how this is being transmitted to a11y, you might find it easy to also put the other flags.

The flags should
a) be localizable, or
b) explicitly not be localized so screen readers could do their own processing and localization. However I'd prefer a) if possible.
One more thing: On Linux, if a message has an attachment, that fact is being spoken by Orca. On Windows, there is no such indication. Joanie, is this Orca processing, or do you simply get that info from Thunderbird? The string is "Has attachment".
Marco, in Linux we're seeing each column as a table cell whose name is derived from the contents. The attachment information has its own column. Therefore when a message has an attachment, the cell in the attachment column contains the name "has attachment" so we present it automatically as part of the row contents.
OK, so we should, in theory, be able to pick up the "replied" and "forwarded" flags just the same.
I just thinking we will add "replied" and/or "forwarder" to accessible name of the cell that represents a subject. That should be ok I think?
Alex, do they not appear in the same place where "read" or "new" would appear? If so, for consistency, we should put them there.
(In reply to comment #11)
> Alex, do they not appear in the same place where "read" or "new" would appear?
> If so, for consistency, we should put them there.
> 

Iirc "read" and "new" are have own cells but "forwarded" and "replied" haven't, they are shown as pictures before subject cell.
The thread pane's columns are as follows:
thread
attachment
subject
junkStatus
sender
recipient
(un)read
date
status
size
flagged (starred)
tags
account (for global inbox)
priority
unread count
total count
location (for cross-folder search)

status is either replied, forwarded, new or unread, whichever comes first.
(In reply to comment #0)
> In the Thunderbird message list, messages that the user has replied to or
> forwarded are indicated by a graphic/icon at the start of the message's
> subject. The presence of this graphic/icon is not being exposed via AT-SPI.
> 

Joanmarie, probably I didn't catch you right and I thought "forwarded" and "replied" information isn't exposed to AT (but it's wrong per Neil's comment above), so do you want to have accessible objects for icons?
(In reply to comment #14)
> (In reply to comment #0)
> > In the Thunderbird message list, messages that the user has replied to or
> > forwarded are indicated by a graphic/icon at the start of the message's
> > subject. The presence of this graphic/icon is not being exposed via AT-SPI.
> Joanmarie, probably I didn't catch you right and I thought "forwarded" and
> "replied" information isn't exposed to AT (but it's wrong per Neil's comment
> above), so do you want to have accessible objects for icons?

That would be preferable I believe. That way, screen readers can decide to filter them out if the user doesn't want them spoken (user verbosity).
Ping. :-)
Note that the status column is optional, and might be turned off by default. If we recommend using the status column as a fix/workaround here then bug 370437 will needs some love.
Agreed re bug 370437. And it does seem to be turned off by default.

With respect to this bug, the question is what to do about the little arrow graphics (that indicate forwarded and replied) that are displayed in the Subject column?
(In reply to comment #18)
> Agreed re bug 370437. And it does seem to be turned off by default.
> 
> With respect to this bug, the question is what to do about the little arrow
> graphics (that indicate forwarded and replied) that are displayed in the
> Subject column?

We could find a way to expose graphics to AT but they won't be helpful unti we expose their semantics as well.

Thinking more I found this be useful in another case. So that it could be used to make more accessible cycle cells. The meantime graphics used to show cycle cell state might be easy recognized by sighted users, however there is no much helpful information for AT users. If we'll take an approach like I suggested in comment #2 then we could fix this. As well this interface can be used to show tooltips on the cell, this should be nice for sighted users. What do you think?
(In reply to comment #19)
> (In reply to comment #18)
> > Agreed re bug 370437. And it does seem to be turned off by default.
> > 
> > With respect to this bug, the question is what to do about the little arrow
> > graphics (that indicate forwarded and replied) that are displayed in the
> > Subject column?

It seems like this is a very small issue since almost every email client rewrites  the subject to include this, however it seems like the screen reader should have access to the data these images provide though I would think most wouldn't use it by default.
  
> We could find a way to expose graphics to AT but they won't be helpful unti we
> expose their semantics as well.

agreed, in atleast this case I don't believe there is a real reason to expose more than the sumantic meaning of the image.

> 
> Thinking more I found this be useful in another case. So that it could be used
> to make more accessible cycle cells. The meantime graphics used to show cycle
> cell state might be easy recognized by sighted users, however there is no much
> helpful information for AT users. If we'll take an approach like I suggested in
> comment #2 then we could fix this. As well this interface can be used to show
> tooltips on the cell, this should be nice for sighted users. What do you think?

makes sense although I'm not familiar with this.
Indeed, Orca does not read "Replied" or "Forwarded" but reads "Starred" and "Attachment" when selecting a mail.
Whiteboard: [bk1]
What's the status on this one?
(In reply to Joanmarie Diggs from comment #22)
> What's the status on this one?

I think one of the thunderbird people (Neil?) needs to decide how they want to expose the summantic information forwarded / replied.  I think I'd be willing to work with any of the approaches mentioned here.
Flags: needinfo?(neil)
As far as I can tell, the status column would be your best bet.
Flags: needinfo?(neil)
(In reply to neil@parkwaycc.co.uk from comment #24)
> As far as I can tell, the status column would be your best bet.

is there some way to get at that data if displaying the column is disabled (which I get the impression from this bug it is by default)?
(In reply to Trevor Saunders from comment #25)
> (In reply to neil@parkwaycc.co.uk from comment #24)
> > As far as I can tell, the status column would be your best bet.
> 
> is there some way to get at that data if displaying the column is disabled
> (which I get the impression from this bug it is by default)?

Sorry, I'm not sure what you're trying to ask here.
If you call GetCellProperties on the row (the thread pane doesn't care which cell) then you'll get all the properties including replied and forwarded.
If you call GetCellText on the status column then you'll get the text whether or not the column is actually visible.
> If you call GetCellProperties on the row (the thread pane doesn't care which
> cell) then you'll get all the properties including replied and forwarded.

using GetRowProperties() feels sort of hacky, since most css stuff people will want to do to rows probably isn't worth exposing.  However exposing some sort of concatonation of them as an object attribute wouldn't be hard.  Screen readers would then have to parse through the set of properties looking for forwarded / replied.

> If you call GetCellText on the status column then you'll get the text
> whether or not the column is actually visible.

so, if the column is not hidden then I would expect we'll put forwarded / replied in the name of the accessible for the message row.  However if its hidden we won't because we ignore hidden columns when computing the name.  I suspect always using hidden columns in computing the name isn't a great idea.
Depends on: 407956
Joanie this should get you an object attribute pair like row-properties "replied forwared blah" on the accessible objects for the row for each message.  I know its kind of gross to look for that, but this is a more general mechanism, it will give you the row properties on a xul tree.
Comment on attachment 749877 [details] [diff] [review]
bug 448235 - expose treeview row properties as row-properties object attribute

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

nsITreeView::GetRowProperties returns a string of space separated row properties, IA2 says:

The proper format for the attributes is
"attribute:value;attribute:subattribute=subvalue,subattribute=subvalue;" 

So it seems IA2 wants to have comma separated string however it doesn't explicitly deny that. It's worth to ask IA2 group.

I'd rename "row-properties" to "props", shorter and no "row" since it's a row accessible.

Do we need to expose cell properties as well?

::: accessible/tests/mochitest/attributes/Makefile.in
@@ +19,5 @@
>  		test_obj_group.xul \
>  		test_obj_group_tree.xul \
>  		test_tag.html \
>  		test_xml-roles.html \
> +		test_tree_properties.xul \

I'd name it test_tree.xul to keep XUL tree specific attributes

::: accessible/tests/mochitest/attributes/test_tree_properties.xul
@@ +47,5 @@
> +    <body xmlns="http://www.w3.org/1999/xhtml">
> +      <a target="_blank"
> +         href="https://bugzilla.mozilla.org/show_bug.cgi?id=503727"
> +         title="Reorganize implementation of XUL tree accessibility">
> +        Mozilla Bug 503727

a copy/paste issue
Assignee: nobody → trev.saunders
(In reply to alexander :surkov from comment #30)
> Comment on attachment 749877 [details] [diff] [review]
> bug 448235 - expose treeview row properties as row-properties object
> attribute
> 
> Review of attachment 749877 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> nsITreeView::GetRowProperties returns a string of space separated row
> properties, IA2 says:
> 
> The proper format for the attributes is
> "attribute:value;attribute:subattribute=subvalue,subattribute=subvalue;" 
> 
> So it seems IA2 wants to have comma separated string however it doesn't
> explicitly deny that. It's worth to ask IA2 group.

seems this is sort of different case since its not subattributes and there values just one big value made up of many whitespace seperated tokens.

> I'd rename "row-properties" to "props", shorter and no "row" since it's a
> row accessible.
> 
> Do we need to expose cell properties as well?

I guess it can't hurt to do them and column ones just for fun.
(In reply to Trevor Saunders (:tbsaunde) from comment #31)
> > nsITreeView::GetRowProperties returns a string of space separated row
> > properties, IA2 says:
> > 
> > The proper format for the attributes is
> > "attribute:value;attribute:subattribute=subvalue,subattribute=subvalue;" 
> > 
> > So it seems IA2 wants to have comma separated string however it doesn't
> > explicitly deny that. It's worth to ask IA2 group.
> 
> seems this is sort of different case since its not subattributes and there
> values just one big value made up of many whitespace seperated tokens.

I asked for clarification on IA2 list (http://lists.linuxfoundation.org/pipermail/accessibility-ia2/2013-May/001811.html)
Comment on attachment 749877 [details] [diff] [review]
bug 448235 - expose treeview row properties as row-properties object attribute

Pete leans towards commas are expected. I'd choose commas too. 

It shouldn't be technically hard so how do you feel about the change?
Attachment #749877 - Flags: review?(surkov.alexander)
(In reply to alexander :surkov from comment #33)
> Comment on attachment 749877 [details] [diff] [review]
> bug 448235 - expose treeview row properties as row-properties object
> attribute
> 
> Pete leans towards commas are expected. I'd choose commas too. 

everywhere or just windows?

> It shouldn't be technically hard so how do you feel about the change?

doing it only for windows might be tricky because other attributes could have spaces that shouldn't be replaced with commas.

I'm not sure I want to use commas everywhere in case someone wants to just read all the properties.
(In reply to Trevor Saunders from comment #34)
> doing it only for windows might be tricky because other attributes could
> have spaces that shouldn't be replaced with commas.
Maybe this used to be possible for custom tree views, but it isn't any more, since the tree painting code expects the row/column/cell properties to be a string of space-delimited words.
(In reply to neil@parkwaycc.co.uk from comment #35)
> (In reply to Trevor Saunders from comment #34)
> > doing it only for windows might be tricky because other attributes could
> > have spaces that shouldn't be replaced with commas.
> Maybe this used to be possible for custom tree views, but it isn't any more,
> since the tree painting code expects the row/column/cell properties to be a
> string of space-delimited words.

but at accessible/src/windows/msaa/AccessibleWrap.cpp:1428 we have no idea the string we're dealing with as an attribute value has anything to do with a tree view, and the things other than tree views are what I"m worried about.
(In reply to Trevor Saunders (:tbsaunde) from comment #34)
> (In reply to alexander :surkov from comment #33)
> > Comment on attachment 749877 [details] [diff] [review]
> > bug 448235 - expose treeview row properties as row-properties object
> > attribute
> > 
> > Pete leans towards commas are expected. I'd choose commas too. 
> 
> everywhere or just windows?

ATK probably wants to have a separate {name, value} pair for each value (https://developer.gnome.org/atk/unstable/AtkText.html#AtkAttribute)

> I'm not sure I want to use commas everywhere in case someone wants to just
> read all the properties.

I'm not sure I follow this

(In reply to Trevor Saunders (:tbsaunde) from comment #36)
> (In reply to neil@parkwaycc.co.uk from comment #35)
> > (In reply to Trevor Saunders from comment #34)
> > > doing it only for windows might be tricky because other attributes could
> > > have spaces that shouldn't be replaced with commas.
> > Maybe this used to be possible for custom tree views, but it isn't any more,
> > since the tree painting code expects the row/column/cell properties to be a
> > string of space-delimited words.
> 
> but at accessible/src/windows/msaa/AccessibleWrap.cpp:1428 we have no idea
> the string we're dealing with as an attribute value has anything to do with
> a tree view, and the things other than tree views are what I"m worried about.

only tree view can provide multiple values for single attribute
(In reply to alexander :surkov from comment #37)
> (In reply to Trevor Saunders (:tbsaunde) from comment #34)
> > (In reply to alexander :surkov from comment #33)
> > > Comment on attachment 749877 [details] [diff] [review]
> > > bug 448235 - expose treeview row properties as row-properties object
> > > attribute
> > > 
> > > Pete leans towards commas are expected. I'd choose commas too. 
> > 
> > everywhere or just windows?
> 
> ATK probably wants to have a separate {name, value} pair for each value
> (https://developer.gnome.org/atk/unstable/AtkText.html#AtkAttribute)

I don't see why what I did falls afoul of that, the attribute's name is  "properties" and its value is something like "selected replied forwared" which afaik is fine.

> > I'm not sure I want to use commas everywhere in case someone wants to just
> > read all the properties.
> 
> I'm not sure I follow this

I mean if screen reader doesn't process attribute value and just reads it all then "forwarded replied" sounds better than "forwarded,replied"

> (In reply to Trevor Saunders (:tbsaunde) from comment #36)
> > (In reply to neil@parkwaycc.co.uk from comment #35)
> > > (In reply to Trevor Saunders from comment #34)
> > > > doing it only for windows might be tricky because other attributes could
> > > > have spaces that shouldn't be replaced with commas.
> > > Maybe this used to be possible for custom tree views, but it isn't any more,
> > > since the tree painting code expects the row/column/cell properties to be a
> > > string of space-delimited words.
> > 
> > but at accessible/src/windows/msaa/AccessibleWrap.cpp:1428 we have no idea
> > the string we're dealing with as an attribute value has anything to do with
> > a tree view, and the things other than tree views are what I"m worried about.
> 
> only tree view can provide multiple values for single attribute

have you checked to make sure no other attributes value can possibly contain spaces?  I sort of thought xml-roles could, and it wouldn't suprise me if there were more...
(In reply to Trevor Saunders (:tbsaunde) from comment #38)
> (In reply to alexander :surkov from comment #37)
> > (In reply to Trevor Saunders (:tbsaunde) from comment #34)
> > > (In reply to alexander :surkov from comment #33)
> > > > Comment on attachment 749877 [details] [diff] [review]
> > > > bug 448235 - expose treeview row properties as row-properties object
> > > > attribute
> > > > 
> > > > Pete leans towards commas are expected. I'd choose commas too. 
> > > 
> > > everywhere or just windows?
> > 
> > ATK probably wants to have a separate {name, value} pair for each value
> > (https://developer.gnome.org/atk/unstable/AtkText.html#AtkAttribute)
> 
> I don't see why what I did falls afoul of that, the attribute's name is 
> "properties" and its value is something like "selected replied forwared"
> which afaik is fine.
> 
> > > I'm not sure I want to use commas everywhere in case someone wants to just
> > > read all the properties.
> > 
> > I'm not sure I follow this
> 
> I mean if screen reader doesn't process attribute value and just reads it
> all then "forwarded replied" sounds better than "forwarded,replied"
> 
> > (In reply to Trevor Saunders (:tbsaunde) from comment #36)
> > > (In reply to neil@parkwaycc.co.uk from comment #35)
> > > > (In reply to Trevor Saunders from comment #34)
> > > > > doing it only for windows might be tricky because other attributes could
> > > > > have spaces that shouldn't be replaced with commas.
> > > > Maybe this used to be possible for custom tree views, but it isn't any more,
> > > > since the tree painting code expects the row/column/cell properties to be a
> > > > string of space-delimited words.
> > > 
> > > but at accessible/src/windows/msaa/AccessibleWrap.cpp:1428 we have no idea
> > > the string we're dealing with as an attribute value has anything to do with
> > > a tree view, and the things other than tree views are what I"m worried about.
> > 
> > only tree view can provide multiple values for single attribute
> 
> have you checked to make sure no other attributes value can possibly contain
> spaces?  I sort of thought xml-roles could, and it wouldn't suprise me if
> there were more...

yeah, a quick look at Accessible::Attributes() shows that if you have a node with role="fo bar" we'll set xml-roles attribute to have value "foo bar" and s/ /,/g there seems wrong.  Even worse if the node has aria-myspecialfoobar="something or other" we'll have attribute myspecialfoobar with value "something or other" and that almost certianly shouldn't have its spaces altered.
(In reply to Trevor Saunders (:tbsaunde) from comment #38)

> > ATK probably wants to have a separate {name, value} pair for each value
> > (https://developer.gnome.org/atk/unstable/AtkText.html#AtkAttribute)
> 
> I don't see why what I did falls afoul of that, the attribute's name is 
> "properties" and its value is something like "selected replied forwared"
> which afaik is fine.

Anyway it's something that ATK guys should confirm.

> > > I'm not sure I want to use commas everywhere in case someone wants to just
> > > read all the properties.
> > 
> > I'm not sure I follow this
> 
> I mean if screen reader doesn't process attribute value and just reads it
> all then "forwarded replied" sounds better than "forwarded,replied"

why?


(In reply to Trevor Saunders (:tbsaunde) from comment #39)
> yeah, a quick look at Accessible::Attributes() shows that if you have a node
> with role="fo bar" we'll set xml-roles attribute to have value "foo bar" and
> s/ /,/g there seems wrong.

again why?

>  Even worse if the node has
> aria-myspecialfoobar="something or other" we'll have attribute
> myspecialfoobar with value "something or other" and that almost certianly
> shouldn't have its spaces altered.

ok, that's a good catch

anyway if a value can contain spaces then AT and we don't have a way to understand what the author meant. So if IA2 format requires us to separate values by commas then we can't do a good job in either case.
Assignee: tbsaunde+mozbugs → nobody
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: