[3.9][a11y] Add landmark to breadcrumbs #23691

wojsmol · 2019-01-27T15:29:09Z

Pull Request for Issue # .

Summary of Changes

added nav tag
added attribute aria-label and role="navigation" to nav tag

Testing Instructions

code inspection
use screen reader (e.g. NVDA) and see that the breadcrumbs is accessible during navigation as a landmark

Similar to PR #23685 for Joomla 4.

Expected result

The 'nav' tag defines one of the main landmarks of the page, and the aria-label attribute makes it possible to distinguish this landmark from other navigation points, e.g. general menu.
Landmarks are supported by assistive technologies. Users of screen readers use them as one of the navigation systems on the page.

Actual result

Assistive technology does not recognize the breadcrumb as a landmark.

Documentation Changes Required

In the Help screens
Include information that the title of the module is used as a label for assistive technologies, so it should be translated (since the title of this module is not displayed, administrators often leave it in English).

zwiastunsw · 2019-01-27T15:49:46Z

I have tested this item ✅ successfully on 7bf3f2f

In my opinion, the role="navigation" attribute is unnecessary. You can leave it. But all popular browsers, including IE11, recognize the nav tag. Nav tag has role=navigation implicite entered. Leaving an attribute, however, will not be an error. In the W3C WAI tutorial, it is still recommended to place a redundant attribute of a role.
See: https://www.w3.org/WAI/tutorials/page-structure/regions/ :

Most current web browsers support the above HTML5 elements and convey the information to assistive technology through the accessibility APIs. However, to maximize compatibility with web browsers and assistive technologies that support WAI-ARIA but do not yet support HTML5, it is currently advisable to use both the HTML5 elements and the corresponding WAI-ARIA roles.

Personally, I am against entering a role attribute. But because we did this in other cases (e.g. in the pagination.php file), let it stay.

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/23691.}

brianteeman · 2019-01-27T18:03:03Z

Sorry but you don't actually understand what the text you are quoting refers to.

The nav element is not exposed in the accessibility tree with internet explorer.

What you are seeing is that NVDA (and perhaps other screen readers have written a workaround to this IE bug) See nvaccess/nvda#6044

This only means that an NVDA and IE11 user is helped. It doesn't fix the bug in IE11 which we have no way of knowing if other assistive technology has their own work around.

So the correct conclusion is to have the role so that ALL users of assistive technology and IE 11 are covered.

We only support specific browsers. We do not support specific assistive technology

zwiastunsw · 2019-01-27T18:28:45Z

I am sorry. I don't know why you assume that I don't understand something. Maybe it' s you who don't understand something?
I wrote what my opinion is. I am free to have my opinion. At the same time I did a successful test.
Please see the new W3C WAI page.
https://www.w3.org/WAI/
Please check in the code that the new HTML5 tags (header, nav, main, footer, etc) are used without the role=" attribute.
Of course, you can say that W3C WAI does wrong. For me this is the correct pattern.
Using these tags (which have an implicitly entered role) without role attributes is correct (and not, as you say, incorrect). Even if some technologies do not support these tags yet.

brianteeman · 2019-01-27T18:33:56Z

Sorry I can't help it if you don't understand the difference between the html 5 specification and the browsers implementation of it and it's the accessibility html5 mapping specification aka html-aam

zwiastunsw · 2019-01-27T18:38:30Z

@brianteeman believe me, I understand.

HLeithner · 2019-01-27T19:55:42Z

This PR adds a feature and may break existing templates without overrides, so this will likely not go into J3.

There is already a J4 PR for this #23685 correct.

brianteeman · 2019-01-27T20:09:49Z

@HLeithner
For 3.x if it. Is changed to div role=navigation it will not break anything and it will be more accessible than it was before

HLeithner · 2019-01-27T20:11:02Z

This PR adds a NAV-tag around the UL-tag and this can break templates.

brianteeman · 2019-01-27T20:12:09Z

Yes which is why I wrote what I wrote

HLeithner · 2019-01-27T20:13:52Z

Maybe break less but why can't it be added to the UL?

brianteeman · 2019-01-27T20:46:06Z

it can be but it would be invalid html

zwiastunsw · 2019-01-27T22:35:36Z

How will adding nav tag damage existing templates? The Breadcrumb is stylized with the breadcrumb class. Of course, it is always possible that some inexperienced webmaster will use some special formatting for the 'nav' tag. But we won't be able to do that.
Adding an attribute role="navigation" to div tag is not an error if you cannot use nav tag (for ex. in HTML4 document), but adding to ul tag is the error! But there is no justification here that excludes the use of nav tag.
Let me also remind you:

6.1.7 Rendered markup
For the time being, rendered markup is not subject to our backwards compatibility promise. We will try not to change markup in such a way that a site might render differently, but we can't promise not to break anything at the present time. We will work on defining ways in which we might make a backwards compatibility promise for markup in the future, but we do not currently have a satisfactory consensus on a workable standard.
https://developer.joomla.org/development-strategy.html

modules/mod_breadcrumbs/tmpl/default.php

brianteeman · 2019-01-28T17:07:07Z

modules/mod_breadcrumbs/tmpl/default.php

+			<?php endif;
+		endforeach; ?>
+	</ul>
+</nav>


Suggested change

</nav>

</div>

Co-Authored-By: wojsmol <wojsmol@wp.pl>

zwiastunsw · 2019-01-28T17:53:39Z

I have tested this item ✅ successfully on f57389c

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/23691.}

brianteeman · 2019-01-28T17:55:15Z

I have tested this item 🔴 unsuccessfully on f57389c

You cant close a div with a nav :(

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/23691.}

brianteeman · 2019-01-28T17:55:53Z

@zwiastunsw how did you successfully test - the markup is wrong

zwiastunsw · 2019-01-28T17:56:41Z

Yes, my mistake.

wojsmol · 2019-01-28T18:11:08Z

@brianteeman @zwiastunsw Issue fixed 😄

zwiastunsw · 2019-01-28T18:11:43Z

I have tested this item ✅ successfully on ede34b4

now is OK

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/23691.}

brianteeman · 2019-01-28T18:12:05Z

I have tested this item ✅ successfully on ede34b4

Thats better - thanks

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/23691.}

Quy · 2019-01-28T18:15:16Z

RTC

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/23691.}

wojsmol · 2019-02-08T11:14:13Z

@HLeithner Please merge this PR, nav was changed to div as requested.

HLeithner · 2019-02-08T17:19:39Z

I requested a different change that seams not to be vaild, thats the reason I didn't merged it.

wojsmol · 2019-02-08T20:53:07Z

@HLeithner This PR now adapt changes suggested ny @brianteeman in #23691 (comment) see https://github.com/joomla/joomla-cms/pull/23691/files?utf8=%E2%9C%93&diff=unified&w=1

HLeithner · 2019-02-09T16:24:59Z

I merge it anyway, thx

wojsmol added 2 commits January 27, 2019 00:29

Add nav landmark to breadcrumbs

267e0da

add role="navigation"

7bf3f2f

joomla-cms-bot added the PR-staging label Jan 27, 2019

brianteeman reviewed Jan 28, 2019

View reviewed changes

modules/mod_breadcrumbs/tmpl/default.php Outdated Show resolved Hide resolved

brianteeman reviewed Jan 28, 2019

View reviewed changes

Update modules/mod_breadcrumbs/tmpl/default.php

f57389c

Co-Authored-By: wojsmol <wojsmol@wp.pl>

closing div

ede34b4

joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Jan 28, 2019

HLeithner merged commit 1ee9472 into joomla:staging Feb 9, 2019

joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Feb 9, 2019

HLeithner added this to the Joomla 3.9.3 milestone Feb 9, 2019

wojsmol deleted the j3_mod_breadcrumbs_a11n branch February 9, 2019 18:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[3.9][a11y] Add landmark to breadcrumbs #23691

[3.9][a11y] Add landmark to breadcrumbs #23691

wojsmol commented Jan 27, 2019

zwiastunsw commented Jan 27, 2019

brianteeman commented Jan 27, 2019

zwiastunsw commented Jan 27, 2019 •

edited

Loading

brianteeman commented Jan 27, 2019

zwiastunsw commented Jan 27, 2019

HLeithner commented Jan 27, 2019

brianteeman commented Jan 27, 2019

HLeithner commented Jan 27, 2019

brianteeman commented Jan 27, 2019

HLeithner commented Jan 27, 2019

brianteeman commented Jan 27, 2019 •

edited

Loading

zwiastunsw commented Jan 27, 2019 •

edited

Loading

brianteeman Jan 28, 2019

zwiastunsw commented Jan 28, 2019

brianteeman commented Jan 28, 2019

brianteeman commented Jan 28, 2019

zwiastunsw commented Jan 28, 2019

wojsmol commented Jan 28, 2019

zwiastunsw commented Jan 28, 2019

brianteeman commented Jan 28, 2019

Quy commented Jan 28, 2019

wojsmol commented Feb 8, 2019

HLeithner commented Feb 8, 2019

wojsmol commented Feb 8, 2019

HLeithner commented Feb 9, 2019

[3.9][a11y] Add landmark to breadcrumbs #23691

[3.9][a11y] Add landmark to breadcrumbs #23691

Conversation

wojsmol commented Jan 27, 2019

Summary of Changes

Testing Instructions

Expected result

Actual result

Documentation Changes Required

zwiastunsw commented Jan 27, 2019

brianteeman commented Jan 27, 2019

zwiastunsw commented Jan 27, 2019 • edited Loading

brianteeman commented Jan 27, 2019

zwiastunsw commented Jan 27, 2019

HLeithner commented Jan 27, 2019

brianteeman commented Jan 27, 2019

HLeithner commented Jan 27, 2019

brianteeman commented Jan 27, 2019

HLeithner commented Jan 27, 2019

brianteeman commented Jan 27, 2019 • edited Loading

zwiastunsw commented Jan 27, 2019 • edited Loading

brianteeman Jan 28, 2019

Choose a reason for hiding this comment

zwiastunsw commented Jan 28, 2019

brianteeman commented Jan 28, 2019

brianteeman commented Jan 28, 2019

zwiastunsw commented Jan 28, 2019

wojsmol commented Jan 28, 2019

zwiastunsw commented Jan 28, 2019

brianteeman commented Jan 28, 2019

Quy commented Jan 28, 2019

wojsmol commented Feb 8, 2019

HLeithner commented Feb 8, 2019

wojsmol commented Feb 8, 2019

HLeithner commented Feb 9, 2019

zwiastunsw commented Jan 27, 2019 •

edited

Loading

brianteeman commented Jan 27, 2019 •

edited

Loading

zwiastunsw commented Jan 27, 2019 •

edited

Loading