Issue #320: Accessibility problems with kallithea and screen readers
| Reported by: | Michael Taboada |
| State: | new |
| Created on: | 2018-05-10 21:50 |
| Updated on: | 2018-06-10 19:29 |
Description
Hi, First off, I am blind, so use a screen reader to access my computer. On linux I use orca, on windows I use NVDA or Jaws for Windows (depending on the thing I'm doing), and on mac I use the built in voice over. The problem is, kallithea has some (relatively major) accessibility problems. I'm going to provide some general problems, and then give some examples of specifics:
Generally, many items are clickable elements, instead of links, and screen readers do not do well with such elements. There are also links that are clickable links, meaning you can press enter on them, and they will load a page, but clicking them either opens a menu under them, or loads a different page. Also, there are some sections where clickables (which should turn into links as with the others) flash on the screen and disappear. This is most prominent with user/group selection fields, where you type a search (a whole name, part of a name, etc) and then click the item you want. The problem here is, I would guess, that upon exiting the field, a search list pops up, but, the screen reader interacts in such a way that the javascript thinks you have moved your mouse away from the list or something so almost immediately closes it. Also, some sections are not labelled properly. This is mostly seen, once again, in the user/group selection UI, though this time in the permission selection (none, read, etc). I also noticed that even well labelled radio buttons (such as the apply to which children selection on repository group permission pages) don't seem to think you've selected them, even when I use the space bar to, so when clicking save it reverts.
Now, some suggestions how to get around these issues:
For clickables, basically the only way to fix them is to make them into a "true" clickable element (meaning an element that is natively accessible with the keyboard without screen reader hacks), such as a button or link. The alternative is to write custom javascript to detect keys being pressed, but let's just not go there. For the links that do different things when clicked, I would say they need to either be re-written to accept a keypress (such as enter or space bar), but only expand, not load the new page. That last part is more of a usability issue, but nevertheless. This would kill two birds with one stone, one being making it much easier for screen reader users to access the menu, another being that instead of having to click (or press a key), load another page, then re-navigate to the same spot where you were, you can just press a key and go. The radio buttons need to either have a label element, or be put in a "true" html table, instead of just text being aligned with css or similar. This would make most screen readers read the proper label, rather than just "radio button" or "<name> radio button". Preferably actually both of these would be done, as tables provide much more structure for the thing that is, essentially, a table for screen readers, and the labels would guarantee that even slightly less well-behaved screen readers would work well with them. As for the flash menu problem, the only real thing I can say there is that the menu can't disappear when it thinks the mouse has been moved off of it, or otherwise it just needs to realize when a screen reader is doing the "moving" (much harder than is necessary imo).
Lastly, for better usability for screen reader users, headings should be added in key places. One big one that is only done on a few places (like a specific repositories page) is to put a heading at the top of the content (above the links for the repository, for example). This is already done in repositories, but almost nowhere else. Other places could include above the settings fields on a setting page, at critical points in a page, etc. It just makes screen reader users' lives much easier as they can press one or two keys to move to the right place, as opposed to having to down arrow 10,20,30+ times.
I will add to this issue if I find anything else, but I think that list gives a pretty good summary of what I found wrong with the accessibility, and more than enough to work on :-).
Thanks!
-Michael.
Attachments
Comments
Comment by Thomas De Schampheleire, on 2018-05-11 19:32
Thanks a lot for taking the time to report these problems.
Are you aware of the version of Kallithea you are using? Is it a release version or directly from the Kallithea repository default branch? Are you deploying Kallithea yourself or is this an instance that someone else is administering and you are just a user of?
The UI between the releases (like the latest release 0.3.4) and the default (development) branch in the Kallithea repository has changed quite extensively, by using the Bootstrap framework. Regardless, I am sure there will be problems remaining and I agree that we should fix them.
Comment by Michael Taboada, on 2018-05-11 20:30
I'm using 0.3.4, and I'm administrating it myself.
I can go ahead and pull the default branch to see what's changed and report back.
Comment by Michael Taboada, on 2018-05-11 21:55
Hi,
I'm actually quite amazed. Upgrading to the latest default branch, the accessibility issues are almost entirely gone. The only remaining one I'm immediately noticing (and it's more an annoyance than a crippling in accessibility), is the headings issue I mentioned. It would be nice if key parts of the page (above the main links in a repository or group, above the settings sections with the name of the settings section like "advanced", etc). I must say the dev version is an absolutely colossal improvement.
Comment by Thomas De Schampheleire, on 2018-05-13 19:21
That is great news, thanks for testing!
I'm trying to understand what you say regarding headings but need more clarification. You mentioned there are some pages/places that do this correctly already. Could you be specific about which pages these are (one or two examples is enough), what the text of these existing headings is, and which is the text of the links that follow? This should help me in understanding what you mean.
Comment by Michael Taboada, on 2018-05-13 21:25
Hi,
So the headings: The pages that do it somewhat correctly are most of the repository view pages, like the summary page, or the settings page. What they do correctly is put a heading with the type and name/group of repository at the top. So it will say like "git group/repository". Then there are links for like settings, summary, switch to..., etc. Below that somewhere I would recommend another heading, which would depend on the page in question. So like for the summary page it might just say summary, and for the settings -> show permissions it might say repository permissions, etc. Basically the point is to be able to jump to important places in the page quickly. This is opposed to what we have to do now for the most part, which is start at the top of the page, arrow down through the main menu links (profile, etc), arrow down through the repository links if applicable, down through the settings sections links, and only then find the actual settings. It could also be helpful for settings pages with many sections to have a heading for each section.
also, as another note, it seems sometimes the radio buttons for the permissions on a group or repository are labelled properly, but sometimes not. In my limited testing the radio buttons seem to be properly labelled when you go to the permissions page, but if you change something and then click save or whatever, it will make them mostly unlabelled. Not quite sure why, but that seems to be my experience. It would also still help to have these elements in an html table for easier navigation for screen reader users.
Comment by Thomas De Schampheleire, on 2018-05-16 20:17
It seems that the headings you refer to are html <nav> elements. Looking at the reference for that element (https://developer.mozilla.org/en-US/docs/Web/HTML/Element/nav) this is specifically intended for navigation blocks. Below is a patch that adds such <nav> elements to typical settings-like pages as you suggested. I did not add any extra text, though, I wonder whether that is mandatory to make it useful for you. Visually there is still something wrong: with this patch the links appear less wide than originally. Would it be possible for you to try this patch?
For non-navigation sections, perhaps you could be helped with <section> tags or other tags, see this page: https://developer.mozilla.org/en-US/docs/Web/Guide/HTML/Content_categories
For the radio buttons there are no labels at all, it seems, at least for the permission pages. They are rendered in a table and the table header has the different values. I was playing to add 'aria-label' elements to them but didn't quite figure out the right syntax yet. To be continued...
Below the patch for the headings.
diff --git a/kallithea/public/less/style.less b/kallithea/public/less/style.less --- a/kallithea/public/less/style.less +++ b/kallithea/public/less/style.less @@ -923,3 +923,7 @@ nav.navbar #quick > li > a, #context-pages > ul > li > a { height: @navbar-height; } + +.settings > nav { + float: left; +} diff --git a/kallithea/templates/admin/my_account/my_account.html b/kallithea/templates/admin/my_account/my_account.html --- a/kallithea/templates/admin/my_account/my_account.html +++ b/kallithea/templates/admin/my_account/my_account.html @@ -21,6 +21,7 @@ ##main <div class="panel-body settings"> + <nav> <ul class="nav nav-pills nav-stacked"> <li class="${'active' if c.active=='profile' else ''}"><a href="${h.url('my_account')}">${_('Profile')}</a></li> <li class="${'active' if c.active=='emails' else ''}"><a href="${h.url('my_account_emails')}">${_('Email Addresses')}</a></li> @@ -30,6 +31,7 @@ <li class="${'active' if c.active=='watched' else ''}"><a href="${h.url('my_account_watched')}">${_('Watched Repositories')}</a></li> <li class="${'active' if c.active=='perms' else ''}"><a href="${h.url('my_account_perms')}">${_('Show Permissions')}</a></li> </ul> + </nav> <div> <%include file="/admin/my_account/my_account_${c.active}.html"/> diff --git a/kallithea/templates/admin/permissions/permissions.html b/kallithea/templates/admin/permissions/permissions.html --- a/kallithea/templates/admin/permissions/permissions.html +++ b/kallithea/templates/admin/permissions/permissions.html @@ -24,11 +24,13 @@ ##main <div class="panel-body settings"> + <nav> <ul class="nav nav-pills nav-stacked"> <li class="${'active' if c.active=='globals' else ''}"><a href="${h.url('admin_permissions')}">${_('Global')}</a></li> <li class="${'active' if c.active=='ips' else ''}"><a href="${h.url('admin_permissions_ips')}">${_('IP Whitelist')}</a></li> <li class="${'active' if c.active=='perms' else ''}"><a href="${h.url('admin_permissions_perms')}">${_('Show Permissions')}</a></li> </ul> + </nav> <div> <%include file="/admin/permissions/permissions_${c.active}.html"/> diff --git a/kallithea/templates/admin/repo_groups/repo_group_edit.html b/kallithea/templates/admin/repo_groups/repo_group_edit.html --- a/kallithea/templates/admin/repo_groups/repo_group_edit.html +++ b/kallithea/templates/admin/repo_groups/repo_group_edit.html @@ -32,11 +32,13 @@ ##main <div class="panel-body settings"> + <nav> <ul class="nav nav-pills nav-stacked"> <li class="${'active' if c.active=='settings' else ''}"><a href="${h.url('edit_repo_group', group_name=c.repo_group.group_name)}">${_('Settings')}</a></li> <li class="${'active' if c.active=='advanced' else ''}"><a href="${h.url('edit_repo_group_advanced', group_name=c.repo_group.group_name)}">${_('Advanced')}</a></li> <li class="${'active' if c.active=='perms' else ''}"><a href="${h.url('edit_repo_group_perms', group_name=c.repo_group.group_name)}">${_('Permissions')}</a></li> </ul> + </nav> <div> <%include file="/admin/repo_groups/repo_group_edit_${c.active}.html"/> diff --git a/kallithea/templates/admin/repos/repo_edit.html b/kallithea/templates/admin/repos/repo_edit.html --- a/kallithea/templates/admin/repos/repo_edit.html +++ b/kallithea/templates/admin/repos/repo_edit.html @@ -20,6 +20,7 @@ ${self.repo_context_bar('options')} <div class="panel panel-primary"> <div class="panel-body settings"> + <nav> <ul class="nav nav-pills nav-stacked"> <li class="${'active' if c.active=='settings' else ''}"> <a href="${h.url('edit_repo', repo_name=c.repo_name)}">${_('Settings')}</a> @@ -43,6 +44,7 @@ <a href="${h.url('edit_repo_statistics', repo_name=c.repo_name)}">${_('Statistics')}</a> </li> </ul> + </nav> <div> <%include file="/admin/repos/repo_edit_${c.active}.html"/> diff --git a/kallithea/templates/admin/settings/settings.html b/kallithea/templates/admin/settings/settings.html --- a/kallithea/templates/admin/settings/settings.html +++ b/kallithea/templates/admin/settings/settings.html @@ -23,6 +23,7 @@ ##main <div class="panel-body settings"> + <nav> <ul class="nav nav-pills nav-stacked"> <li class="${'active' if c.active=='vcs' else ''}"><a href="${h.url('admin_settings')}">${_('VCS')}</a></li> <li class="${'active' if c.active=='mapping' else ''}"><a href="${h.url('admin_settings_mapping')}">${_('Remap and Rescan')}</a></li> @@ -33,6 +34,7 @@ <li class="${'active' if c.active=='search' else ''}"><a href="${h.url('admin_settings_search')}">${_('Full Text Search')}</a></li> <li class="${'active' if c.active=='system' else ''}"><a href="${h.url('admin_settings_system')}">${_('System Info')}</a></li> </ul> + </nav> <div> <%include file="/admin/settings/settings_${c.active}.html"/> diff --git a/kallithea/templates/admin/user_groups/user_group_edit.html b/kallithea/templates/admin/user_groups/user_group_edit.html --- a/kallithea/templates/admin/user_groups/user_group_edit.html +++ b/kallithea/templates/admin/user_groups/user_group_edit.html @@ -25,6 +25,7 @@ ##main <div class="form panel-body settings"> + <nav> <ul class="nav nav-pills nav-stacked"> <li class="${'active' if c.active=='settings' else ''}"><a href="${h.url('edit_users_group', id=c.user_group.users_group_id)}">${_('Settings')}</a></li> <li class="${'active' if c.active=='advanced' else ''}"><a href="${h.url('edit_user_group_advanced', id=c.user_group.users_group_id)}">${_('Advanced')}</a></li> @@ -32,6 +33,7 @@ <li class="${'active' if c.active=='default_perms' else ''}"><a href="${h.url('edit_user_group_default_perms', id=c.user_group.users_group_id)}">${_('Show Permissions')}</a></li> <li class="${'active' if c.active=='members' else ''}"><a href="${h.url('edit_user_group_members', id=c.user_group.users_group_id)}">${_('Show Members')}</a></li> </ul> + </nav> <div> <%include file="/admin/user_groups/user_group_edit_${c.active}.html"/> diff --git a/kallithea/templates/admin/users/user_edit.html b/kallithea/templates/admin/users/user_edit.html --- a/kallithea/templates/admin/users/user_edit.html +++ b/kallithea/templates/admin/users/user_edit.html @@ -25,6 +25,7 @@ ##main <div class="panel-body settings"> + <nav> <ul class="nav nav-pills nav-stacked"> <li class="${'active' if c.active=='profile' else ''}"><a href="${h.url('edit_user', id=c.user.user_id)}">${_('Profile')}</a></li> <li class="${'active' if c.active=='emails' else ''}"><a href="${h.url('edit_user_emails', id=c.user.user_id)}">${_('Emails')}</a></li> @@ -33,6 +34,7 @@ <li class="${'active' if c.active=='advanced' else ''}"><a href="${h.url('edit_user_advanced', id=c.user.user_id)}">${_('Advanced')}</a></li> <li class="${'active' if c.active=='perms' else ''}"><a href="${h.url('edit_user_perms', id=c.user.user_id)}">${_('Show Permissions')}</a></li> </ul> + </nav> <div> <%include file="/admin/users/user_edit_${c.active}.html"/> diff --git a/kallithea/templates/summary/summary.html b/kallithea/templates/summary/summary.html --- a/kallithea/templates/summary/summary.html +++ b/kallithea/templates/summary/summary.html @@ -119,6 +119,7 @@ </div> </div> </div> + <nav> <ul id="summary-menu-stats" class="list-group pull-right"> <li class="list-group-item"> <a title="${_('Owner')} ${c.db_repo.owner.email}"> @@ -164,6 +165,7 @@ </li> %endif </ul> + </nav> </div> </div>
Comment by Michael Taboada, on 2018-05-16 22:05
Hi,
I will see if I can try this patch out. However, the headings I was referring to were standard h1, h2, etc headings. I will test to see if this works, but I kind of suspect it won't since screen readers treat nav and h1/2 differently in most cases.
Comment by Thomas De Schampheleire, on 2018-05-17 07:16
The thing is that there are no headings on any of the places that you referred to. For example the 'repo type / repo name' heading on repository pages are just standard text elements inside a <nav> tag.
Comment by Michael Taboada, on 2018-05-30 13:25
Hi,
I tried out the patch. Not sure if it was something my screen reader was doing or what (since I just upgraded it recently) but the navs now show properly as navs, and don't act like headings. What I would do is put headings above or somewhere near the navs (since apparently the navs aren't acting like headings anymore to my screen reader).
Also another note, I've noticed a few more unlabelled buttons. Mostly these are the expand/collapse buttons including the one in the repository navigation that has settings, follow/unfollow, etc under it, and the one that has the login menu, or user links if logged in.
Comment by Thomas De Schampheleire, on 2018-06-10 19:29
Hi Michael,
I tried to use Orca to be able to experience it myself, but failed. It installs fine, but I can't figure out how to make it read a page. Is it correct that Orca is just sitting in the background once started? I tried to map a shortcut on 'Read entire document', then go to Firefox and press it, but nothing happens. How is it supposed to work?
Below is another patch just to understand if it behaves better for you, could you try it? It is supposed to add a label 'Main navigation' on the main navigation section (the one with links to Repositories, Public Journal, Gists, etc.), also add a 'landmark' to the repository name/type section on a repository page, and another landmark on such pages at the repository navigation section. On each of these landmarks a label is added that should be visible to your screenreader ("Repository metadata" and "Repository navigation" respectively).
Could you let me know if these concepts would improve things for you? If they do, then I can go further and apply them in more places.
Thanks!
diff --git a/kallithea/templates/base/base.html b/kallithea/templates/base/base.html --- a/kallithea/templates/base/base.html +++ b/kallithea/templates/base/base.html @@ -95,7 +95,7 @@ <!--- CONTEXT BAR --> <nav id="context-bar" class="navbar navbar-inverse"> <div class="container-fluid"> - <div class="navbar-header"> + <div class="navbar-header" role="navigation" aria-label="Repository metadata"> <div class="navbar-brand"> ${repolabel(c.db_repo)} @@ -123,7 +123,7 @@ </button> </div> <div id="context-pages" class="navbar-collapse collapse"> - <ul class="nav navbar-nav navbar-right"> + <ul class="nav navbar-nav navbar-right" role="navigation" aria-label="Repository navigation"> <li class="${'active' if current == 'summary' else ''}" data-context="summary"><a href="${h.url('summary_home', repo_name=c.repo_name)}"><i class="icon-doc-text"></i>${_('Summary')}</a></li> %if rev: <li class="${'active' if current == 'changelog' else ''}" data-context="changelog"><a href="${h.url('changelog_file_home', repo_name=c.repo_name, revision=rev, f_path='')}"><i class="icon-clock"></i>${_('Changelog')}</a></li> diff --git a/kallithea/templates/base/root.html b/kallithea/templates/base/root.html --- a/kallithea/templates/base/root.html +++ b/kallithea/templates/base/root.html @@ -115,7 +115,7 @@ <%block name="head_extra"/> </head> <body> - <nav class="navbar navbar-inverse mainmenu"> + <nav class="navbar navbar-inverse mainmenu" aria-label="Main navigation"> <div class="navbar-header" id="logo"> <a class="navbar-brand" href="${h.url('home')}"> <span class="branding">${c.site_name}</span>