Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HTML validation and accessibility errors in some themes #11242

Open
DemeSzabolcs opened this issue Feb 22, 2022 · 6 comments
Open

HTML validation and accessibility errors in some themes #11242

DemeSzabolcs opened this issue Feb 22, 2022 · 6 comments
Milestone

Comments

@DemeSzabolcs
Copy link
Contributor

Describe the bug

There are accessibility problems in:

Asserting the accessibility analysis result failed. Check the accessibility report failure dump for details.
-------- Shouldly.ShouldAssertException : axeResult.Violations
    should be empty but had
1
    item and was
[Selenium.Axe.AxeResultItem (66331987)]

Additional Info:
Links must have discernible text: 
    <a class="btn btn-dark btn-social mx-2" href="<https://www.twitter.com/@">>
                        <i class="fab fa-twitter"></i>
                    </a>
    <a class="btn btn-dark btn-social mx-2" href="<https://www.facebook.com/@">>
                        <i class="fab fa-facebook"></i>
                    </a>
    <a class="btn btn-dark btn-social mx-2" href="<https://www.linkedin.com/in/@">>
                        <i class="fab fa-linkedin"></i>
                    </a>
    <a class="btn btn-dark btn-social mx-2" href="<https://www.twitter.com/@">>
                        <i class="fab fa-twitter"></i>
                    </a>
    <a class="btn btn-dark btn-social mx-2" href="<https://www.facebook.com/@">>
                        <i class="fab fa-facebook"></i>
                    </a>
    <a class="btn btn-dark btn-social mx-2" href="<https://www.linkedin.com/in/@">>
                        <i class="fab fa-linkedin"></i>
                    </a>
    <a class="btn btn-dark btn-social mx-2" href="<https://www.twitter.com/@">>
                        <i class="fab fa-twitter"></i>
                    </a>
    <a class="btn btn-dark btn-social mx-2" href="<https://www.facebook.com/@">>
                        <i class="fab fa-facebook"></i>
                    </a>
    <a class="btn btn-dark btn-social mx-2" href="<https://www.linkedin.com/in/@">>
                        <i class="fab fa-linkedin"></i>
                    </a>
    <a href="#">
                    <img class="img-fluid img-brand d-block mx-auto" src="/media/logos/microsoft.svg" alt="">
                </a>
    <a href="#">
                    <img class="img-fluid img-brand d-block mx-auto" src="/media/logos/google.svg" alt="">
                </a>
    <a href="#">
                    <img class="img-fluid img-brand d-block mx-auto" src="/media/logos/facebook.svg" alt="">
                </a>
    <a href="#">
                    <img class="img-fluid img-brand d-block mx-auto" src="/media/logos/ibm.svg" alt="">
                </a>
    <a class="btn btn-dark btn-social mx-2" href="#!"><i class="fab fa-twitter" aria-hidden="true"></i></a>
    <a class="btn btn-dark btn-social mx-2" href="#!"><i class="fab fa-facebook-f" aria-hidden="true"></i></a>
    <a class="btn btn-dark btn-social mx-2" href="#!"><i class="fab fa-linkedin-in" aria-hidden="true"></i></a>

Also there are elements with insufficient color contrasts, but I didn't include those errors. For those see: #11241

Asserting the accessibility analysis result failed. Check the accessibility report failure dump for details.
-------- Shouldly.ShouldAssertException : axeResult.Violations
    should be empty but had
1
    item and was
[Selenium.Axe.AxeResultItem (14851760)]

Additional Info:
    Links must have discernible text: 
    <a class="btn btn-dark m-3" href="<https://twitter.com">>
            <i class="fab fa-twitter"></i>
        </a>
    <a class="btn btn-dark m-3" href="<https://facebook.com">>
            <i class="fab fa-facebook-f"></i>
        </a>
    <a class="btn btn-dark m-3" href="<https://instagram.com">>
            <i class="fab fa-instagram"></i>
        </a>

There are HTML validation errors in:

  " Check the HTML validation report in the failure dump for details.
-------- Shouldly.ShouldAssertException : validationResult.Output
    should be empty but was
"12bbd89b-7645-413b-a1d3-d69909ac40b9.html
  25:55   error  The autoplay attribute is not allowed on <video>  no-autoplay
  31:320  error  Attribute "method" has invalid value "POST"       attribute-allowed-values

Ôťľ 2 problems (2 errors, 0 warnings)

More information:
  https://html-validate.org/rules/no-autoplay.html
  https://html-validate.org/rules/attribute-allowed-values.html
"

To Reproduce

Steps to reproduce the behavior:

  1. Set up Orchard Core with the agency recipe/coming soon recipe.
  2. Login and go to the homepage.
  3. See that there are accessibility errors and/or HTML validation errors on the given site.

Expected behavior

After setting up Orchard Core with the agency recipe/coming soon recipe on the home page, there should be no accessibility and/or HTML validation problems.

@hishamco
Copy link
Member

As @Skrypt said earlier we are not the creators of the themes and we should use theme as it is

@Piedone
Copy link
Member

Piedone commented Feb 22, 2022

While with #11241 I kind of agree that it's better to fix in the theme (since the fix is not just a technical one but affects the design), here I don't see why we'd need to keep these errors and wait for an update in the original project: The Liquid code is a derivative work, not part of the original template but part of Orchard, and the fixes are solely technical. We can fix them in Orchard and at the same time let the original author now about them to fix them in the original code too.

Let me follow up under the PR.

@Piedone
Copy link
Member

Piedone commented Mar 11, 2022

There's a new issue in the Blog theme, duplicated fas class:

image

@Piedone
Copy link
Member

Piedone commented Mar 11, 2022

And there's a new accessibility issue with The Default Theme too. This is an HTML file, just rename to .html, but GitHub doesn't allow uploading them.
AccessibilityReport.txt

@Skrypt
Copy link
Contributor

Skrypt commented Mar 11, 2022

<i class="fas {{ Model.ContentItem.Content.Category.Icon.Text }} fa-xs" aria-hidden="true"></i>

Remove the first "fas" since this seems to be added from database with the IconPicker. But maybe it was added for backward compatibility when we switched to a new version of FontAwesome for those who we're using "fa" instead of "fas". So, if that's the case then the issue is that the IconPicker saves "fas" in the database.

@sebastienros
Copy link
Member

Just fix what is specifically added by us, or file issues in the official repos with PR if possible. Then we'll update the templates.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants