Closed
Bug 955087
Opened 10 years ago
Closed 10 years ago
Improve keyboard accessibility of the contact list
Categories
(Instantbird Graveyard :: Contacts window, defect)
Instantbird Graveyard
Contacts window
Tracking
(Not tracked)
RESOLVED
FIXED
1.3
People
(Reporter: aleth, Assigned: aleth)
Details
Attachments
(1 file, 7 obsolete files)
15.15 KB,
patch
|
florian
:
review+
|
Details | Diff | Splinter Review |
*** Original post on bio 1658 at 2012-08-21 22:07:00 UTC *** As seen in https://bugzilla.mozilla.org/show_bug.cgi?id=778709 for the status selector: makes the focus on the status selector visible (important when tabbing through the controls of the window) and lets the user open the status type popup with the down arrow while the status selector is focused.
Assignee | ||
Comment 1•10 years ago
|
||
*** Original post on bio 1658 as attmnt 1829 at 2012-08-22 17:07:00 UTC *** - Makes avatar focusable and changeable via keyboard - Remove focusability from conversations on hold when that listbox is empty - Add focus borders to all focusable elements that didn't show their state - The down arrow on the status selector now opens the status selection popup Might need followups on OS != Linux (certainly for Win-aero).
Attachment #8353588 -
Flags: review?(florian)
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → aleth
Status: NEW → ASSIGNED
Assignee | ||
Updated•10 years ago
|
Summary: Make focus visible on all editable elements and improve status selector keyboard accessibility → Improve keyboard accessibility of the contact list
Comment 2•10 years ago
|
||
Comment on attachment 8353588 [details] [diff] [review] Patch *** Original change on bio 1658 attmnt 1829 at 2012-08-29 22:10:38 UTC *** I think I would prefer if the status type icon wasn't focusable, and using the down arrow key to open the status type popup when the status message is focused. For the status message and the display name's focus rings, I suggest doing this for Mac: #statusMessage:-moz-focusring:not([editing]) { %ifdef XP_MACOSX margin: 30px -2px -1px; border: 2px solid rgba(0,0,0,0.15); border-radius: 5px; %else margin: 31px -1px -1px; border: dotted 1px -moz-dialogtext; %endif } #displayName:-moz-focusring:not([editing]) { %ifdef XP_MACOSX margin: -1px -1px 16px -2px; border: 2px solid rgba(0,0,0,0.15); border-radius: 5px; %else margin: -1px -1px 16px; border: dotted 1px -moz-dialogtext; %endif } I first tried with -moz-focus-ring as the color but it looks really ugly above a toolbar background without a real textbox. Mic may be able to help you for the Windows theming and/or you may be able to re-use things from https://bugzilla.mozilla.org/attachment.cgi?id=650953&action=diff
Attachment #8353588 -
Flags: review?(florian) → review-
Comment 3•10 years ago
|
||
*** Original post on bio 1658 at 2012-08-29 22:39:58 UTC *** (In reply to comment #2) > I first tried with -moz-focus-ring as the color I meant -moz-mac-focusring sorry.
Assignee | ||
Comment 4•10 years ago
|
||
*** Original post on bio 1658 at 2012-08-30 00:30:04 UTC *** (In reply to comment #2) > I think I would prefer if the status type icon wasn't focusable, and using the > down arrow key to open the status type popup when the status message is > focused. I am a bit worried this will not be discoverable. Who is going to guess this (for accessibility and otherwise)? This is no problem for TB where there is a downward chevron, http://www.h-online.com/imgs/43/9/1/0/5/5/1/2012-08-24-13-05-57-5f7cfe-63b2430d93d20b8d.png
Assignee | ||
Comment 5•10 years ago
|
||
*** Original post on bio 1658 at 2012-08-30 16:03:59 UTC *** (In reply to comment #4) > I am a bit worried this will not be discoverable. Who is going to guess this > (for accessibility and otherwise)? clokep also thinks it's pretty standard. James, is there a way to indicate this (in effect 'you can press the down arrow to change status') via a11y flags? Or is that not necessary?
Comment 6•10 years ago
|
||
*** Original post on bio 1658 by James Teh <jamie AT nvaccess.org> at 2012-08-31 00:20:56 UTC *** (In reply to comment #5) > James, is there a way to indicate this (in effect 'you can press the down arrow > to change status') via a11y flags? Or is that not necessary? It should have an accessible role of button (in v1.2, it's link) and it should expose the hasPopup state. If this isn't happening already, you can achieve it with attributes role="button" aria-haspoup="true". Note that the way Gecko exposes this is a bit ambiguous; it's not clear whether it's a button that opens a menu when pressed or a button with a secondary menu (as in this case), but exposing this is better than nothing.
Assignee | ||
Comment 7•10 years ago
|
||
*** Original post on bio 1658 as attmnt 1880 at 2012-09-01 12:59:00 UTC *** OK, so this - adds/removes the a11y roles when appropriate - changes the status popup in the way flo suggested - changes the status message focus ring to incorporate the statustype icon to match this behaviour. - ensures (when possible) that there is always a listitem selected when tabbing through the contact list via the keyboard. (I did not want to do this generally whenever the contact list has focus because it looks ugly.)
Attachment #8353638 -
Flags: review?(florian)
Attachment #8353638 -
Flags: feedback?(bugzilla)
Assignee | ||
Comment 8•10 years ago
|
||
Comment on attachment 8353588 [details] [diff] [review] Patch *** Original change on bio 1658 attmnt 1829 at 2012-09-01 12:59:45 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353588 -
Attachment is obsolete: true
Assignee | ||
Comment 9•10 years ago
|
||
*** Original post on bio 1658 at 2012-09-01 13:13:58 UTC ***
> Mic may be able to help you for the Windows theming and/or you may be able to
> re-use things from
> https://bugzilla.mozilla.org/attachment.cgi?id=650953&action=diff
Since I can't test this anyway, I think one might as well do this in a follow-up bug. It's not like anything will break beyond the existing brokenness while those rules are absent.
Comment 10•10 years ago
|
||
*** Original post on bio 1658 by James Teh <jamie AT nvaccess.org> at 2012-09-03 05:30:11 UTC *** (In reply to comment #1) > - Makes avatar focusable and changeable via keyboard I assume this is @id="userIcon". Some issues: 1. This is unlabelled according to accessibility. Perhaps it could be given a label like "Change Icon" or similar. If you don't want this to be visible and you have no other way of doing it, use the aria-label attribute. 2. How often is this likely to be used by keyboard users? Would it make more sense to include it in the Contact context menu (thus freeing up a tab stop)? The tab order is hardly unmanageable right now, so it's not really important, but thought I'd ask. 3. Pressing down arrow on this button opens the status type menu. This doesn't make sense to me, but perhaps I'm missing something. > - Remove focusability from conversations on hold when that listbox is empty Nice. Works well. Here's str for one other bug: 1. Tab to the status type button. 2. Press enter. 3. Press escape. 4. Press down arrow to open the menu. 5. Press escape to close it. Expected: Focus should be on the status type button. Actual: Focus is on the avatar button.
Assignee | ||
Comment 11•10 years ago
|
||
Comment on attachment 8353638 [details] [diff] [review] Patch v2 *** Original change on bio 1658 attmnt 1880 at 2012-09-03 09:34:24 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353638 -
Flags: review?(florian)
Attachment #8353638 -
Flags: feedback?(bugzilla)
Assignee | ||
Comment 12•10 years ago
|
||
*** Original post on bio 1658 as attmnt 1892 at 2012-09-03 10:02:00 UTC *** Fixes the issues reported by Jamie.
Attachment #8353650 -
Flags: review?(florian)
Attachment #8353650 -
Flags: feedback?(bugzilla)
Assignee | ||
Comment 13•10 years ago
|
||
Comment on attachment 8353638 [details] [diff] [review] Patch v2 *** Original change on bio 1658 attmnt 1880 at 2012-09-03 10:02:30 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353638 -
Attachment is obsolete: true
Comment 14•10 years ago
|
||
Comment on attachment 8353650 [details] [diff] [review] Patch *** Original change on bio 1658 attmnt 1892 by jamie AT nvaccess.org at 2012-09-03 10:52:18 UTC *** The issues I mentioned are fixed. Thanks! Unfortunately, I discovered one more (my apologies for not spotting it the first time): The aria-haspopup attribute should be removed (along with the role attribute) when editing the status message. Of course, it should be reinstated when editing is done. Marking f+, as this will be great with the above fix.
Attachment #8353650 -
Flags: feedback?(bugzilla) → feedback+
Comment 15•10 years ago
|
||
*** Original post on bio 1658 by James Teh <jamie AT nvaccess.org> at 2012-09-03 10:54:50 UTC *** It's also worth noting that only enter works to activate these controls; space doesn't work. This may confuse some keyboard users, as space normally activates buttons. I'm guessing this is just the default behaviour of these particular XUL controls and it's a very minor point; I've seen other buttons that don't respond to space either, so many users may try both.
Assignee | ||
Comment 16•10 years ago
|
||
*** Original post on bio 1658 as attmnt 1893 at 2012-09-03 10:56:00 UTC *** Right, I should have spotted that too!
Attachment #8353651 -
Flags: review?(florian)
Assignee | ||
Comment 17•10 years ago
|
||
Comment on attachment 8353650 [details] [diff] [review] Patch *** Original change on bio 1658 attmnt 1892 at 2012-09-03 10:56:58 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353650 -
Attachment is obsolete: true
Attachment #8353650 -
Flags: review?(florian)
Assignee | ||
Comment 18•10 years ago
|
||
*** Original post on bio 1658 at 2012-09-03 11:00:16 UTC *** (In reply to comment #12) > It's also worth noting that only enter works to activate these controls; space > doesn't work. This may confuse some keyboard users, as space normally activates > buttons. I'm guessing this is just the default behaviour of these particular > XUL controls and it's a very minor point; I've seen other buttons that don't > respond to space either, so many users may try both. Actually it could easily be added that pressing space also triggered editing the display name/status message. But I'm not sure that "feels right"? Another alternative would be to start the editing whenever the user starts typing when the element has focus.
Assignee | ||
Comment 19•10 years ago
|
||
*** Original post on bio 1658 as attmnt 1894 at 2012-09-03 12:02:00 UTC *** It turns out the documentation for role=button explicitly states that one should add a handler for the space key to match the usual behaviour of buttons.
Attachment #8353652 -
Flags: review?(florian)
Assignee | ||
Comment 20•10 years ago
|
||
Comment on attachment 8353651 [details] [diff] [review] Patch *** Original change on bio 1658 attmnt 1893 at 2012-09-03 12:02:48 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353651 -
Attachment is obsolete: true
Attachment #8353651 -
Flags: review?(florian)
Assignee | ||
Comment 21•10 years ago
|
||
*** Original post on bio 1658 as attmnt 1895 at 2012-09-03 18:53:00 UTC *** Update OS X CSS rule (please test) OK, hopefully this is it ;)
Attachment #8353653 -
Flags: review?(florian)
Assignee | ||
Comment 22•10 years ago
|
||
Comment on attachment 8353652 [details] [diff] [review] Patch *** Original change on bio 1658 attmnt 1894 at 2012-09-03 18:53:15 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353652 -
Attachment is obsolete: true
Attachment #8353652 -
Flags: review?(florian)
Comment 23•10 years ago
|
||
Comment on attachment 8353653 [details] [diff] [review] Patch (reactivated) *** Original change on bio 1658 attmnt 1895 at 2012-10-02 22:06:49 UTC *** > statusMessageKeyPress: function bl_statusMessageKeyPress(aEvent) { >+ case aEvent.DOM_VK_TAB: >+ if (aEvent.shiftKey) >+ break; >+ // Ensure some item is selected when navigating by keyboard. >+ if (!this.selectFirstItem("convlistbox")) >+ this.selectFirstItem("buddylistbox"); So here you try both lists... >+ userIconKeyPress: function bl_userIconKeyPress(aEvent) { >+ case aEvent.DOM_VK_TAB: >+ if (!aEvent.shiftKey) >+ break; >+ // Ensure a contact is selected when navigating by keyboard. >+ this.selectFirstItem("buddylistbox"); ... but here only one. Is there a reason for this difference? > keyPress: function bl_keyPress(aEvent) { > let target = aEvent.target; > while (target && target.localName != "richlistbox") > target = target.parentNode; >+ if (aEvent.keyCode == aEvent.DOM_VK_TAB) { >+ // Ensure some item is selected when navigating by keyboard. >+ if (target.id == "convlistbox" && !aEvent.shiftKey) Nit: remove the duplicated space here. I haven't tried the patch, but the code looks good otherwise.
Attachment #8353653 -
Flags: review?(florian) → review-
Assignee | ||
Comment 24•10 years ago
|
||
*** Original post on bio 1658 at 2012-10-10 19:41:27 UTC *** (In reply to comment #17) > Comment on attachment 8353653 [details] [diff] [review] (bio-attmnt 1895) [details] > Patch > > > statusMessageKeyPress: function bl_statusMessageKeyPress(aEvent) { > So here you try both lists... > ... but here only one. > > Is there a reason for this difference? TAB from the status message goes to the conversations on hold if there are any, to the contacts if not. Shift-TAB from the user icon is the reverse direction and always goes to the contacts.
Comment 25•10 years ago
|
||
*** Original post on bio 1658 at 2012-10-10 21:34:35 UTC *** (In reply to comment #18) > > Is there a reason for this difference? > TAB from the status message goes to the conversations on hold if there are any, > to the contacts if not. Shift-TAB from the user icon is the reverse direction > and always goes to the contacts. Is this based on the assumption that if the contact list is empty, there can't be any conversation on hold? If so, that's frequently wrong if you put conversations on hold (or use hide auto-join), didn't check "show offline buddies" and switch to the offline status.
Assignee | ||
Comment 26•10 years ago
|
||
*** Original post on bio 1658 at 2012-10-10 21:39:41 UTC *** (In reply to comment #19) > If so, that's frequently wrong if you put conversations on hold (or use hide > auto-join), didn't check "show offline buddies" and switch to the offline > status. Good catch. Of course offline *conversations* aren't hidden.
Assignee | ||
Comment 27•10 years ago
|
||
*** Original post on bio 1658 as attmnt 1944 at 2012-10-10 21:44:00 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353701 -
Flags: review?(florian)
Assignee | ||
Comment 28•10 years ago
|
||
Comment on attachment 8353653 [details] [diff] [review] Patch (reactivated) *** Original change on bio 1658 attmnt 1895 at 2012-10-10 21:44:27 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353653 -
Attachment is obsolete: true
Comment 29•10 years ago
|
||
Comment on attachment 8353701 [details] [diff] [review] Patch with fixes *** Original change on bio 1658 attmnt 1944 at 2012-10-10 23:37:06 UTC *** This new attachment lost the ifdefs of skin/blist.css, and added unrelated changes in conversation.xml and Bubbles.
Attachment #8353701 -
Flags: review?(florian) → review-
Assignee | ||
Comment 30•10 years ago
|
||
*** Original post on bio 1658 at 2012-10-11 15:22:43 UTC *** (In reply to comment #22) > This new attachment lost the ifdefs of skin/blist.css, and added unrelated > changes in conversation.xml and Bubbles. Hrm, not sure what happened there. Those unrelated changes are old too. Sorry.
Assignee | ||
Comment 31•10 years ago
|
||
Comment on attachment 8353701 [details] [diff] [review] Patch with fixes *** Original change on bio 1658 attmnt 1944 at 2012-10-11 16:03:05 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353701 -
Attachment is obsolete: true
Assignee | ||
Comment 32•10 years ago
|
||
Comment on attachment 8353653 [details] [diff] [review] Patch (reactivated) *** Original change on bio 1658 attmnt 1895 at 2012-10-11 16:10:26 UTC *** (In reply to comment #19) > > > Is there a reason for this difference? > > TAB from the status message goes to the conversations on hold if there are any, > > to the contacts if not. Shift-TAB from the user icon is the reverse direction > > and always goes to the contacts. > Is this based on the assumption that if the contact list is empty, there can't > be any conversation on hold? Ah! I remember now why I did it this way at the time. The code you are pointing at ensures that a listitem is always selected (if one exists) when you tab to the listbox part of the contact window. The difference arises because "conversations on hold" is not in the tab order when there are no conversations on hold, whereas the contact listbox always is. (And we can't just skip an empty contact list, since the context menu must remain accessible.) So, reactivating patch (latest-1) (which still has a surplus space nit) for now.
Attachment #8353653 -
Attachment description: Patch → Patch (reactivated)
Attachment #8353653 -
Attachment is obsolete: false
Assignee | ||
Comment 33•10 years ago
|
||
*** Original post on bio 1658 as attmnt 1975 at 2012-10-17 15:28:00 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353734 -
Flags: review?(florian)
Assignee | ||
Comment 34•10 years ago
|
||
Comment on attachment 8353653 [details] [diff] [review] Patch (reactivated) *** Original change on bio 1658 attmnt 1895 at 2012-10-17 15:28:36 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353653 -
Attachment is obsolete: true
Comment 35•10 years ago
|
||
Comment on attachment 8353734 [details] [diff] [review] Patch with nit fix *** Original change on bio 1658 attmnt 1975 at 2012-10-27 03:12:43 UTC *** >diff --git a/chrome/en-US/locale/en-US/instantbird/instantbird.dtd b/chrome/en-US/locale/en-US/instantbird/instantbird.dtd >index d8ac207..b02a593 100644 >--- a/chrome/en-US/locale/en-US/instantbird/instantbird.dtd >+++ b/chrome/en-US/locale/en-US/instantbird/instantbird.dtd >@@ -52,19 +52,19 @@ > <!ENTITY conversation.contextMenu.close "Close Tab"> > <!ENTITY chat.participants "Participants:"> > > <!ENTITY errorConsoleCmd.label "Error Console"> > <!ENTITY errorConsoleCmd.accesskey "C"> > <!ENTITY errorConsoleCmd.commandkey "j"> > > <!ENTITY preferencesCmd.label "Options..."> >-<!ENTITY preferencesCmd.accesskey "O"> >+<!ENTITY preferencesCmd.accesskey "O"> > <!ENTITY preferencesCmdUnix.label "Preferences"> >-<!ENTITY preferencesCmdUnix.accesskey "n"> >+<!ENTITY preferencesCmdUnix.accesskey "n"> This whitespace-only change doesn't apply. (In reply to comment #24) > Ah! I remember now why I did it this way at the time. > [...] we can't just skip an > empty contact list, since the context menu must remain accessible.) Ok. Thanks for the clarification.
Attachment #8353734 -
Flags: review?(florian) → review+
Comment 36•10 years ago
|
||
*** Original post on bio 1658 at 2012-10-27 03:37:03 UTC *** http://hg.instantbird.org/instantbird/rev/93d1283e4719
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.3
You need to log in
before you can comment on or make changes to this bug.
Description
•