Accessibility improvements for Extensions in Razor Markup #9576

RachBreeze · 2020-12-17T15:41:58Z

This PR is to resolve #9321

It is important for accessibility purposes that the correct natural language is defined on the page via the <HTML lang=""> element this allows screen readers and other assistive technology use the correct pronunciation when reading the page content (as shown here) https://www.youtube.com/watch?v=QwOoU8T24UY&feature=youtu.be

Where a property on a page hasn't been defined in the user's selected language Umbraco allows the content to fall back as detailed here https://our.umbraco.com/Documentation/Getting-Started/Design/Rendering-Content/#using-fall-back-methods this means that the language defined in the HTML element is potentially not the language of the content in that property.

W3C details more about this issue here https://www.w3.org/International/questions/qa-html-language-declarations#:~:text=Quick%20answer,an%20element%20surrounding%20that%20content. This PR aims to both support the accessibility requirement following the pattern defined in thisarticle in the section "What if there's no element to hang your attribute on?" It also gives the developer the facility to use their discretion as to whether the code should display <span lang="fallbackLanguage">...</span> around the property.

I have added "Fallback.DisplayFallBackLanguage" as an optional add an additional optional parameter to Fallback.To to support this.

The need for this is based on the EU funded research project for improving the process of creating accessible content by authors

Merge latest Umbraco v8 into my repo

…into v8/contrib # Please enter a commit message to explain why this merge is necessary, # especially if it merges an updated upstream into a topic branch. # # Lines starting with '#' will be ignored, and an empty message aborts # the commit.

…nto v8/contrib

glombek

Hi @RachBreeze,

Thanks for this. It works for me (mostly!)

However, I've found a few things that don't quite work seamlessly.

If I have a rich text editor property and then use ModelsBuilder to obtain the value (which will be an IHtmlString), the GetMarkUpForFallbackLanguage method fails, as it relies on the type parameter being a string (or being able to be implicitly cast to from a string), which IHtmlString is not. This will also likely fail for any other property types (ie int).

System.InvalidCastException: 'Invalid cast from 'System.String' to 'System.Web.IHtmlString'.'

Secondly, if the type is a string. the result will need to be output wrapped in an Html.Raw statement to escape the markup. This isn't ideal UX-wise but also poses a security risk as the initial text is not HTML escaped. Unfortunately, it may be suboptimal to simply escape the initial text as this may not be desired by the developers.

Boy, this is a bit of a mine-field, eh?!

I wonder if it might be worth making this a completely different method rather than trying to force it into the Value / ValueFor implementation? I agree something like this is necessary (the workaround at the moment is painful!) but I'm not sure this is the optimal solution either.

Happy to talk through this some time (it may also be worth us chatting to HQ to discuss options here).

Joe

src/Umbraco.Core/Models/PublishedContent/Fallback.cs

src/Umbraco.Web/Models/PublishedContent/PublishedValueFallback.cs

…existing code.

RachBreeze · 2021-01-04T11:20:58Z

Hi @glombek
Thank you for taking time to review.

The return of the needs to be part of the fallback options, as it's only the request to use a fall back that means the language could change, and the user would need to be notified of those change.

I would need guidance on any new implementations for this. But am happy to do any required changes. It's worth noting that @nul800sebastiaan did advise on the original implementation #9321.

I did realise that the code would need wrapping in Html.Raw but this is how content in the RTE has to be rendered. As you say however, any new implementation for fallback could consider that too.

I have now added type testing to GetMarkUpForFallbackLanguage this allows us to determine if the span needs adding, for example just because the user has fallen back to a different value of an int, the pronunciation of that int value will not change. The only time language needs to change is if the requested value contains text. This allowed me to resolve the IHtmlString issue too.

RachBreeze · 2021-04-18T15:04:24Z

Heya I've found another demonstration of why this change is important https://vimeo.com/210258413

This fixes the "string" vs "String" issue in the previous commit and prevents the function returning invalid markup

glombek · 2021-04-18T19:14:05Z

Hi @RachBreeze, I tested your changes today and found a couple of errors. I hope you don't mind but I took the liberty of fixing them for you as they were pretty minor.

I think you've nailed the functionality now for different property types and the fallback of simply returning the initial value instead of falling over is much neater.

I also fixed the breaking test and merged the latest version of v8/contrib in to resolve conflicts.

You'll notice in my changes that I didn't use the is type checking for IHtmlString as because it's an interface we'd need to do some costly reflection to return the output, so it's a little bit of a compromise but works well.

If you're happy with my changes we can progress.

Thanks!

RachBreeze · 2021-04-24T08:31:58Z

Hi @glombek the changes look good thank you

RachBreeze · 2021-06-27T10:16:09Z

Good morning
I hope you're well, is there anything else I can help with for progressing this PR?

glombek · 2021-07-06T17:43:52Z

Thanks @RachBreeze - no further action needed on your part, just me you were waiting for!

That's merged now and you can expect to see it in 8.16.

RachBreeze · 2021-07-07T07:25:02Z

Thank you

nul800sebastiaan · 2021-08-04T12:40:21Z

Hi @RachBreeze and @glombek - unfortunately based on some feedback from the team I have reverted this change for the 8.16 release, I'll quote some of the conversation here:

The main issue is that it would need to change the returned value from a simple string (that's automatically HTML encoded) into an HTML string, so the added <span/<div lang= is rendered correctly, but then

the actual value isn't HTML encoded

this is only done when the fallback is used, so when it's not, the actual string value is returned (which gets automatically HTML encoded), but otherwise you'd want to skip HTML encoding (otherwise the span/div would end up displayed as text)

If the value is a simple string, it can conditionally return HTML (for the added span/div, if the fallback to a different language is used) and you don't know when that's the case... So you'd probably end up always rendering it as raw HTML, opening up to XSS issues.
Meaning the HTML can only reliably be added when the property return type is already IHtmlString (but even then it's a bit strange it's adding a div/block level element).

A better and more generic way would be to expose the used fallback and it's 'options', e.g. the returned culture for the ToLanguage fallback, the used content node for ToAncestors or a boolean indicating if the returned value is from the defaultValue parameter for ToDefaultValue.

I hope this will help with a re-take of this PR for a future version @RachBreeze but please do ask if you need clarification.

Sorry I had to revert this one after it was merged so long ago, but I hope you'll understand we want to do the right thing here. We totally love the idea of this feature of course and we want it, but in a different way. 😅 Thanks so much for your efforts so far and again, let us know if you need some more guidance!

)" This reverts commit a1e0af6.

RachBreeze and others added 9 commits August 22, 2020 16:15

Merge pull request #1 from umbraco/v8/contrib

3a031e8

Merge latest Umbraco v8 into my repo

Merge branch 'v8/contrib' of https://github.com/umbraco/Umbraco-CMS i…

0b49cd0

…nto v8/contrib

Merge branch 'v8/contrib' of https://github.com/umbraco/Umbraco-CMS i…

e395a42

…nto v8/contrib

Merge branch 'v8/contrib' of https://github.com/umbraco/Umbraco-CMS i…

d988bc6

…nto v8/contrib

Merge branch 'v8/contrib' of https://github.com/umbraco/Umbraco-CMS i…

35b9897

…nto v8/contrib

Merge branch 'v8/contrib' of https://github.com/umbraco/Umbraco-CMS i…

81d9665

…nto v8/contrib

Merge branch 'v8/contrib' of https://github.com/umbraco/Umbraco-CMS i…

5d2e0b5

…nto v8/contrib

Support for language of parts

5eef043

glombek suggested changes Dec 31, 2020

View reviewed changes

glombek added the state/hq-discussion label Dec 31, 2020

Rachel Breeze added 2 commits January 4, 2021 09:05

Changed casing of "FallBack" to "Fallback" to be consistent with the …

cb10e0b

…existing code.

Now tests the type to determine if span should be added.

ff3e199

RachBreeze mentioned this pull request Jan 18, 2021

Indicate fallback language to the browser when used from Model.Value #9321

Closed

nul800sebastiaan added state/needs-investigation This requires input from HQ or community to proceed and removed state/hq-discussion labels Mar 25, 2021

Joe Glombek added 3 commits April 18, 2021 18:16

Merge branch 'v8/contrib' into pr/9576

0efcdd5

Remove magic strings and return <div> for IHtmlString values

d8b48f9

This fixes the "string" vs "String" issue in the previous commit and prevents the function returning invalid markup

Fix for test providing string as "object"

f84e820

glombek approved these changes Apr 19, 2021

View reviewed changes

glombek merged commit a1e0af6 into umbraco:v8/contrib Jul 6, 2021

glombek added release/8.16.0 category/accessibility community/pr and removed state/needs-investigation This requires input from HQ or community to proceed labels Jul 6, 2021

glombek added the type/feature label Jul 6, 2021

nul800sebastiaan removed the release/8.16.0 label Aug 4, 2021

nul800sebastiaan added a commit that referenced this pull request Aug 4, 2021

Revert "Accessibility improvements for Extensions in Razor Markup (#9576

96d5935

)" This reverts commit a1e0af6.

RachBreeze mentioned this pull request Sep 26, 2022

ATAG - Feature - Change language umbraco/Umbraco-CMS.Accessibility.Issues#74

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Accessibility improvements for Extensions in Razor Markup #9576

Accessibility improvements for Extensions in Razor Markup #9576

RachBreeze commented Dec 17, 2020 •

edited

Loading

glombek left a comment

RachBreeze commented Jan 4, 2021

RachBreeze commented Apr 18, 2021

glombek commented Apr 18, 2021

RachBreeze commented Apr 24, 2021

RachBreeze commented Jun 27, 2021

glombek commented Jul 6, 2021

RachBreeze commented Jul 7, 2021

nul800sebastiaan commented Aug 4, 2021 •

edited

Loading

Accessibility improvements for Extensions in Razor Markup #9576

Accessibility improvements for Extensions in Razor Markup #9576

Conversation

RachBreeze commented Dec 17, 2020 • edited Loading

glombek left a comment

Choose a reason for hiding this comment

RachBreeze commented Jan 4, 2021

RachBreeze commented Apr 18, 2021

glombek commented Apr 18, 2021

RachBreeze commented Apr 24, 2021

RachBreeze commented Jun 27, 2021

glombek commented Jul 6, 2021

RachBreeze commented Jul 7, 2021

nul800sebastiaan commented Aug 4, 2021 • edited Loading

RachBreeze commented Dec 17, 2020 •

edited

Loading

nul800sebastiaan commented Aug 4, 2021 •

edited

Loading