Closed Bug 953853 Opened 10 years ago Closed 10 years ago

[Accessibility] Add accessible text for the status icons and other purely graphical info

Categories

(Instantbird Graveyard :: Contacts window, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: bugzilla, Assigned: benediktp)

Details

Attachments

(1 file, 4 obsolete files)

*** Original post on bio 412 by Marco Zehe (:MarcoZ) <marco.zehe AT googlemail.com> at 2010-05-05 09:47:00 UTC ***

The buddy list currently only renders the buddy name or display name to screen readers. Information such as the status etc. are not available to blind users of Instantbird.

The buddies are "buddy" items, which inherit from richlistitem. The richlistitem has a property called label which collects all the text from the child labels of a richlistitem. However, since buddy uses graphics to represent some of the information, this property needs to be overridden. For the original, see:
http://mxr.mozilla.org/mozilla1.9.2/source/toolkit/content/widgets/richlistbox.xml#530

So in addition to what's already there, localizable status messages such as "group", "online/available" etc. must be exposed as textual information in this string. This string is made up for the accessibility APIs only, so the formatting can be anything useful (like prepended, or appended in parenthesis after the name (preferable)) etc.
*** Original post on bio 412 at 2010-05-05 10:34:16 UTC ***

Bug 953660 (bio 214) is about missing accessible information on buddies on the buddy list. Thanks for pointing out how to solve this problem properly :)
*** Original post on bio 412 as attmnt 325 at 2010-08-05 20:55:00 UTC ***

This is an early patch. Would require localizability and a solution for the groups names. Seems to be easy to be too wordy on the label, we should be pretty selective what we put there as it might get read out over and over again (e.g. protocol type or group names).

I had a discussion about this on #accessibility btw, just ask me if interested.
Assignee: nobody → benediktp
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
*** Original post on bio 412 at 2010-08-05 21:08:35 UTC ***

Remark: I tested it with accprobe ofcourse.
*** Original post on bio 412 by Parham Doustdar <parham90 AT gmail.com> at 2011-03-31 10:35:49 UTC ***

(In reply to comment #3)
> Remark: I tested it with accprobe ofcourse.


Hi,

I tested this using the latest nightly build of Instantbird, but the problem is still there; all the children of richlistitem (namely the "expand" button, and the "start a conversation" button are also there in the item label). I suppose something like:

<display name> (<status> <protocol>) <status message>

might be good initially, however, I'd say it'd be better to make it editable, so that the user can configure what and in what order they want to listen to.
*** Original post on bio 412 at 2011-03-31 12:05:02 UTC ***

(In reply to comment #2)
> This is an early patch. Would require localizability and a solution for the
> groups names. Seems to be easy to be too wordy on the label, we should be
> pretty selective what we put there as it might get read out over and over again


(In reply to comment #4)
> (In reply to comment #3)
> might be good initially, however, I'd say it'd be better to make it editable,
> so that the user can configure what and in what order they want to listen to.

That could be the best thing to do. People seem to have pretty different ideas what should be there and in which order (not being too verbose and not offering too little information, ..)

I wonder how we should treat contacts (groupings of buddies).
*** Original post on bio 412 by Parham Doustdar <parham90 AT gmail.com> at 2011-03-31 13:07:09 UTC ***

(In reply to comment #5)
> That could be the best thing to do. People seem to have pretty different ideas
> what should be there and in which order (not being too verbose and not offering
> too little information, ..)
> 

Yes. I'd say making it a string based on a variable-like approach would be best. For example, if I want to hear something like, "Parham (online) Yep, I'm online!", the configuration string would be "%name% (%status%) %message%". This leaves the user able to have variables such as %groupname%, %protocol%, and so forth.

> I wonder how we should treat contacts (groupings of buddies).

There is a few ways, but I would say the best is to go with a treeview approach. The roots of the tree are groups (buddies, other contacts, etc), and buddies are the leaves of the tree. However, contacts are branches that have all the buddies that are actually grouped together as leaves.
*** Original post on bio 412 at 2011-11-06 14:44:26 UTC ***

I'm not working on any of these -> unassigning from myself so I don't block anyone who'd be willing to take one of them.
*** Original post on bio 412 by Peter Vagner <pvdeejay AT gmail.com> at 2011-11-12 18:20:48 UTC ***

Hello,
I think non configurable basic info is still better than too verbose unnecessary info.
I don't have a proper patch but I've just quickly adapted this for the current nightly of Instantbird 1.2 a1 pre.

I think visually you can't arrange contact list columns at the moment thus by adding this people with low vision will get similar experience to what their sighted folks do have.

For average users you need to put this into the contact.xml file above the first occurence of </implementation>


     <property name="label" readonly="true">
        <getter>
          <![CDATA[
            const XULNS =
              "http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul";

            var labelText = "";

            let status    = this.getAttribute("status");
            let statusmsg = this.contact.statusText;
            labelText = this.contact.displayName + " (" + status + (statusmsg ? ": " + statusmsg: "") +  ")";
            return labelText;
          ]]>
        </getter>
      </property>
*** Original post on bio 412 at 2012-02-10 10:11:51 UTC ***

What should we do here?
I'm a sighted user, have read the comments again and still don't know what's the best thing to have. Use the short solution that Peter Vagner provided or maybe the customizable string? Or maybe the latter should be left to an add-on?
*** Original post on bio 412 by Parham Doustdar <parham90 AT gmail.com> at 2012-02-10 12:50:27 UTC ***

Other than the statusmsg message being out of the parentheses, I am perfectly fine with what Peter suggested. In other words:

            labelText = this.contact.displayName + " (" + status + ") " + statusmsg;

This is just because a text with parentheses is generally easier to comprehend for the brain. :-)

Thanks Peter and Benedikt!
Attached patch Patch v1 (obsolete) — Splinter Review
*** Original post on bio 412 as attmnt 1239 at 2012-03-09 23:24:00 UTC ***

This patch sets the label to "Contact name (Status - Status Message)" for normal (not expanded) contacts; for expanded contacts, the label will read "Contact name (Status - Status Message): buddy name 1, buddy name 2, ..., buddy name N".

For sighted users, the buddies show their repective status messages too on the contact list. Should these messages (and additional information like the protocol/network name of each buddy?) also be included in the label?
Comment on attachment 8352066 [details] [diff] [review]
A small patch implementing the property 'label' of richlistitems

*** Original change on bio 412 attmnt 325 at 2012-03-09 23:24:26 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8352066 - Attachment is obsolete: true
*** Original post on bio 412 at 2012-03-10 00:21:36 UTC ***

This patch looks fine, and I guess it's all the information that needs to be included...but I'm not expert on accessibility.
*** Original post on bio 412 by Peter Vagner <pvdeejay AT gmail.com> at 2012-03-10 06:58:46 UTC ***

I believe this is fine. Status info about subcontacts while the main contact is expanded is not needed as we'll need to file seperate issue for that.
Ideally it would be nice to be able to focus items representing subcontacts in order to be able to call their popup menu.
I don't know how this might be done though.
Comment on attachment 8352992 [details] [diff] [review]
Patch v1

*** Original change on bio 412 attmnt 1239 at 2012-03-10 09:23:09 UTC ***

We can wait a while for more feedback if wanted, requesting review anyways.
Attachment #8352992 - Flags: review?(clokep)
Comment on attachment 8352992 [details] [diff] [review]
Patch v1

*** Original change on bio 412 attmnt 1239 at 2012-03-10 13:00:13 UTC ***

This looks fine to me, my only other question would be about whether it needs to be, and if it can be, localized.
Attachment #8352992 - Flags: review?(clokep) → review+
*** Original post on bio 412 by Peter Vagner <pvdeejay AT gmail.com> at 2012-03-10 15:26:59 UTC ***

have I overlooked something?
label property only fetches details about the contact without adding any hardcoded strings. What should be localized?
*** Original post on bio 412 at 2012-03-10 16:06:02 UTC ***

(In reply to comment #16)
> have I overlooked something?
> label property only fetches details about the contact without adding any
> hardcoded strings. What should be localized?

I asked Patrick on IRC what he meant with that and it was about the question if some locales might want to have the information in a different order or with other separators between buddy names for example.
*** Original post on bio 412 by Peter Vagner <pvdeejay AT gmail.com> at 2012-03-10 16:14:18 UTC ***

I think locales don't swap tooltips either. So trying to pair the experience with what the sighted friends do have we are fine now. At least this is how I feel it.
*** Original post on bio 412 at 2012-03-11 17:12:15 UTC ***

Comment on attachment 8352992 [details] [diff] [review] (bio-attmnt 1239)
Patch v1

>diff -r 5540a627ace2 instantbird/content/contact.xml

>@@ -543,6 +554,21 @@
>       </xul:vbox>
>     </content>
>     <implementation>
>+     <!-- Textual representation of the contact, its status and grouped buddies
>+          for accessibility tools -->
>+     <property name="label" readonly="true">
>+       <getter>
>+       <![CDATA[
>+         let accessLabel = this.contact.displayName +
>+                           " (" + this.getAttribute("statusText") + "): ";
>+         let buddies = this.contact.getBuddies();
>+         for (let i = 0; i < buddies.length; i++)
>+           accessLabel += (i != 0 ? ", " : "") + buddies[i].userName;
>+         return accessLabel;

Did you mean?
  accessLabel += this.contact.getBuddies().map(function(b) b.userName).join(", ");

In reply to comments 15-17, I think rtl locales may want to change the order, but we don't have any rtl locale now, so let's not worry about it for now :).
*** Original post on bio 412 by Peter Vagner <pvdeejay AT gmail.com> at 2012-03-15 06:54:21 UTC ***

I would rather change the recently discussed line to:
  accessLabel += this.contact.getBuddies().map(function(b) b.displayName).join(",");

because userName gives the protocol dependant userName like ICQ uin for ICQ, facebook JID for facebook contacts and so on.

Anyway this is also set for the collapsed contacts. so if doable it would be better to include only subcontact names besides the main contact.
Can this be done please?
*** Original post on bio 412 at 2012-03-15 10:19:08 UTC ***

(In reply to comment #20)
> I would rather change the recently discussed line to:
>   accessLabel += this.contact.getBuddies().map(function(b)
> b.displayName).join(",");

Sounds good, as the displayName is what we display anyway.

> include only subcontact names besides the main contact.

I don't understand what you mean with this sentence.
*** Original post on bio 412 by Peter Vagner <pvdeejay AT gmail.com> at 2012-03-15 15:00:15 UTC ***

Currently this patch is supposed to format label in the following way
displayName (status): status text, subcontact1, subcontact2, subcontact3 n....

It would be nice if the main contact wouldn't be included twice once at the beginning and second time after a status message.
*** Original post on bio 412 at 2012-03-20 22:33:47 UTC ***

(In reply to comment #22)
> Currently this patch is supposed to format label in the following way
> displayName (status): status text, subcontact1, subcontact2, subcontact3 n....
> 
> It would be nice if the main contact wouldn't be included twice once at the
> beginning and second time after a status message.

It's the most available buddy that lends its name to the contact, not necessarily the first in this list.
*** Original post on bio 412 by Peter Vagner <pvdeejay AT gmail.com> at 2012-03-21 06:23:11 UTC ***

I see it might be difficult to code then. This is just a gimmick so we can live without it perhaps.
But in such case it would be nice to somehow supress the contact name at the end for contacts where there are no subcontacts.
By reading the patch it appears there should be seperate entry for the simple contacts, not meta contacts but aafter manually applying the patch it does not work this way.
Attached patch Updated patch (obsolete) — Splinter Review
*** Original post on bio 412 as attmnt 1357 at 2012-04-14 17:58:00 UTC ***

This patch uses map/join now for creating the list of buddies.
Attachment #8353110 - Flags: review?(clokep)
Comment on attachment 8352992 [details] [diff] [review]
Patch v1

*** Original change on bio 412 attmnt 1239 at 2012-04-14 17:58:28 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8352992 - Attachment is obsolete: true
Comment on attachment 8353110 [details] [diff] [review]
Updated patch

*** Original change on bio 412 attmnt 1357 at 2012-04-17 16:16:09 UTC ***

>diff --git a/instantbird/content/contact.xml b/instantbird/content/contact.xml
>--- a/instantbird/content/contact.xml
>+++ b/instantbird/content/contact.xml
>@@ -538,16 +549,30 @@
>           </xul:hbox>
>         </xul:vbox>
>       </xul:hbox>
>       <xul:vbox anonid="contactBuddies" class="contactBuddies">
>         <children/>
>       </xul:vbox>
>     </content>
>     <implementation>
>+     <!-- Textual representation of the contact, its status and grouped buddies
>+          for accessibility tools -->
>+     <property name="label" readonly="true">
>+       <getter>
>+       <![CDATA[
>+         let accessLabel = this.contact.displayName +
>+                           " (" + this.getAttribute("statusText") + "): ";
>+         accessLabel += this.contact.getBuddies()
>+                            .map(function(b) b.displayName).join(",");
Why bother putting this on a separate line? Also, I think you want .join(", ").

>+         return accessLabel;
(In fact you could probably not even store the value and just return it...).
Attachment #8353110 - Flags: review?(clokep) → review-
*** Original post on bio 412 at 2012-04-17 16:21:16 UTC ***

Should this use localized formatted strings? I'm not sure if it's relevant for screen readers, but if that string was displayed in the UI, I would definitely want to add a space between ) and : in the French locale.
*** Original post on bio 412 by Peter Vagner <pvdeejay AT gmail.com> at 2012-04-17 18:13:17 UTC ***

I know i said we can live without but more and more i am thinking about it i would like the addition proposed in comments 22 to 24. Can you consider it if possible?

Regardind localization i agree with florian.
*** Original post on bio 412 at 2012-04-18 13:24:48 UTC ***

(In reply to comment #24)

> it would be nice to somehow supress the contact name at the
> end for contacts where there are no subcontacts.

I think I agree with this. If we assume that this label will only be used for screenreaders and that people who need to use a screenreader can't use a mouse, they will never have more than one buddy per contact, so no subcontacts is the general case, not a specific case.
*** Original post on bio 412 by Peter Vagner <pvdeejay AT gmail.com> at 2012-04-18 21:52:46 UTC ***

Hello,
I'm afraid you've missunderstood me.
Eventhough I am blind and it's a bit difficult to setup I still use and like the meta contacts feature. I have several contacts including more than  one buddy.
The ideal solution for user's perspective would be to be able to expand a contact and focus  its individual buddies as seperate items - the way currently groups are expanded and the contacts inside can be focused. Expanded / collapsed state needs to be communicated to the AT's using aria properties. However this is complex and should be handled as a seperate ticket.
For now we do have two choices:
1) Include only main contact name in this label
2) include the main name at the beginning of a label and include all other buddies excluding the main name at the end of the label.
*** Original post on bio 412 at 2012-04-18 22:10:05 UTC ***

(In reply to comment #30)

> Eventhough I am blind and it's a bit difficult to setup I still use and like
> the meta contacts feature.

I wonder how you managed to do that.

> For now we do have two choices:
> 1) Include only main contact name in this label
> 2) include the main name at the beginning of a label and include all other
> buddies excluding the main name at the end of the label.

There's no "main name", as Mic tried to explain in comment 23, so I don't see 2) as an option.

I would say there's also:
3) include the contact's display name at the beginning, and only if it's a meta contact, the list of buddy display names at the end.
*** Original post on bio 412 by Peter Vagner <pvdeejay AT gmail.com> at 2012-04-19 02:48:14 UTC ***

Some screenreaders have features which can emulate mouse features. this is limited comparing it to the real mouse usage but thanks to the nature of mozilla's platform in instant bird the following can be accomplished emulating the mouse:
- focus the contact you want to drag,
- use screenreader functionality to move the mouse to the item having system focus,
- use screenreader functionality which locks left mouse button pressed down until you use another functionality to unlock it,
- move the system focus to the contact you wish to drop the first contact to,
- expand it using the right arrow key,
- use the screen reader functionality to review the expanded contact aas the place where one is supposed to drop the other contaact is not focusable,
- once the proper place is being reviewed you can use screenreader functionality to route the mouse where the screenreader specific review points to,
- finally use screenreader functionality to unlock the locked down left mouse button

So you can see in eight steps I can create a meta contact. Compare how convenient this might be for a sighted users.

I see there is no main contact instead  always this is the most available contact. What I am trying to achieve that the most recent contact is not going to be reported multiple times. once at the beginning of a label and second time at the end of a label in the list of all subcontacts.
So now we need to decide if you are going to implement my crazy idea reffered to as point 2 in my prior comment, or we'll stick with point 3 which I an also accept.
*** Original post on bio 412 at 2012-04-19 07:44:29 UTC ***

OK, I'll go with suggestion 3) then. I also attempted to make the label localizable even though I couldn't test that yet because I don't remember how to check that using DOMi. Maybe I need to install accprobe again.
*** Original post on bio 412 at 2012-04-19 09:35:59 UTC ***

(In reply to comment #33)
> OK, I'll go with suggestion 3) then. I also attempted to make the label
> localizable even though I couldn't test that yet because I don't remember how
> to check that using DOMi.

On the right pane, use the "JavaScript Object" view.
*** Original post on bio 412 at 2012-04-19 09:39:19 UTC ***

(In reply to comment #32)
 
> So you can see in eight steps I can create a meta contact. Compare how
> convenient this might be for a sighted users.

It's not really convenient for sighted users, as while keeping the mouse button pressed there's no way to scroll the list, so creating a meta contact with a contact that's at the end of the list and one that's at the beginning of the list is almost impossible. I think this issue doesn't exist with your method.
Attached patch Patch v5 (obsolete) — Splinter Review
*** Original post on bio 412 as attmnt 1517 at 2012-05-23 10:56:00 UTC ***

I made the strings localizable and the buddies are hidden for the expanded view now if there's only one for this contact.
Attachment #8353269 - Flags: review?(clokep)
Comment on attachment 8353110 [details] [diff] [review]
Updated patch

*** Original change on bio 412 attmnt 1357 at 2012-05-23 10:56:30 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353110 - Attachment is obsolete: true
Comment on attachment 8353269 [details] [diff] [review]
Patch v5

*** Original change on bio 412 attmnt 1517 at 2012-05-23 12:00:07 UTC ***

>diff --git a/instantbird/content/contact.xml b/instantbird/content/contact.xml

>@@ -538,16 +562,39 @@
>     <implementation>
>+     <!-- Textual representation of the contact and its status (and grouped
>+          buddies if applicable) for accessibility tools -->
>+     <property name="label" readonly="true">
>+       <getter>
>+       <![CDATA[
>+         let buddies = this.contact.getBuddies().map(function(b) b.displayName);
I think this is only used in one place below, in which case you don't need a separate variable for it.

>+         // Show simple formatting if there's only one buddy.
>+         if (buddies.length == 1) {
>+           let params = [this.contact.displayName, this.getAttribute("statusText")];
>+           return this.bundle.formatStringFromName("contact.collapsed", params,
>+                                                   params.length);
>+         }
>+
>+         buddies = buddies.join(this.bundle.GetStringFromName("contact.buddySeparator"));
>+         let params = [this.contact.displayName, this.getAttribute("statusText"),
>+                       buddies];
Don't repeat this, define params above the if statement and then push buddies into the array.

>+         return this.bundle.formatStringFromName("contact.expanded", params,
>+                                                 params.length);
You could probably define the entity name as a variable too so you're not repeating this line, but I won't r- for that.

>+       ]]>
>+       </getter>
>+     </property>

>diff --git a/instantbird/locales/en-US/chrome/instantbird/instantbird.properties b/instantbird/locales/en-US/chrome/instantbird/instantbird.properties
>+contact.buddySeparator=,
Should this be a comma followed by a space? (Maybe your editor stripped the space?)
Attachment #8353269 - Flags: review?(clokep) → review-
Attached patch PatchSplinter Review
*** Original post on bio 412 as attmnt 1519 at 2012-05-23 15:22:00 UTC ***

Good review comments, thanks a lot! The code looks much nicer now.
I also (re-)added the space and comment in the properties file. No idea how it got lost, I had it in all iterations but this one :(

The array "buddies" is needed to tell their number before they're joined and pushed into the params array by the way.
Attachment #8353271 - Flags: review?(clokep)
Comment on attachment 8353269 [details] [diff] [review]
Patch v5

*** Original change on bio 412 attmnt 1517 at 2012-05-23 15:22:09 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353269 - Attachment is obsolete: true
Comment on attachment 8353271 [details] [diff] [review]
Patch

*** Original change on bio 412 attmnt 1519 at 2012-05-23 15:25:10 UTC ***

Looks a lot more readable. Thanks! :)
Attachment #8353271 - Flags: review?(clokep) → review+
Whiteboard: [checkin-needed]
*** Original post on bio 412 at 2012-05-24 17:43:43 UTC ***

http://hg.instantbird.org/instantbird/rev/3a7a56b260e9

Thanks Mic!
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Whiteboard: [checkin-needed]
Target Milestone: --- → 1.2
You need to log in before you can comment on or make changes to this bug.