OrchardCMS / OrchardCore

Orchard Core is an open-source modular and multi-tenant application framework built with ASP.NET Core, and a content management system (CMS) built on top of that framework.
https://orchardcore.net
BSD 3-Clause "New" or "Revised" License
7.22k stars 2.34k forks source link

Orchard Core Admin HTML not following valid HTML requirements. #12271

Open Psichorex opened 1 year ago

Psichorex commented 1 year ago

Describe the bug

Orchard Core Admin HTML templates are not following html regulations. As a result you can come across failing UI tests due to html validations.

To Reproduce

Steps to reproduce the behavior: Run any html validators on any OC Admin page. Most likely it will find errors. E.g. element must have a lang attribute.

Expected behavior

There shouldn't be invalid html in Orchard Core.

Screenshots

Here you can see a UI Test failing due to html validation. image image image

Full validation report provided by the UI test.

HtmlValidationReport.txt

Piedone commented 1 year ago

Note that people here don't know about what you do with UI testing. Focus on the what the issue is in Orchard, exactly (elaborate on what you show in the screenshot, everything else is not interesting for others here).

Psichorex commented 1 year ago

@Piedone Will elaborate on this more today. FYI I think this error is coming from our extensions. I think the old navigators that are for default prefix disable these html validations while GoToRelativeUrl doesn't. I will update here asap.

hishamco commented 1 year ago

I'm suggesting to move this into Lombiq UI Testing ToolBox repo, unless there's an issue related to Orchard Core

Psichorex commented 1 year ago

@hishamco Well there are 2 approaches for this issue. 1) Orchard Core HTML templates are not following HTML validation rules thus this problem is related to Orchard Core. 2) We don't care about OC not following these and Lombiq UI Testing Toolbox should just neglect these validations.

It's up to the community which route shall we take.

Piedone commented 1 year ago

It's an issue with the validity of the HTML of the Orchard admin. Please edit the issue so it's actually about that, not about UI tests, since that's not relvant.

hishamco commented 1 year ago

Orchard Core HTML templates are not following HTML validation rules thus this problem is related to Orchard Core

@Psichorex could you please elaborate which rules are you talking about, or you mean all of the HTML validation rules?

Psichorex commented 1 year ago

@hishamco There are too many to name. Probably not all but several rules were neglected when constructing the HTML of Orchard Admin. E.g. html-has-lang , rules about <button> elements and so on. I can't name them all. Basically the Orchard Admin pages' html should be presented to an html validator and that would be able to filter out all the missing stuff.

hishamco commented 1 year ago

We could try to use an HTML validation service, unless @Piedone prefer another solution because already he working on Lombiq UI Testing ToolBox

Skrypt commented 1 year ago

Here is one that should be used : https://accessibilityinsights.io/docs/web/overview/

hishamco commented 1 year ago

Can we setup a tenant on try.orchardcore.com to test the admin site

Piedone commented 1 year ago

@Psichorex it's exactly three issues with a button on the admin ;). Please just add the output of the HTML validator to the issue.

Psichorex commented 1 year ago

@Piedone Here it's 3. But when I was adding UI testing to OC Commerce the BasicFeaturesTests produces much more similar ones.

Piedone commented 1 year ago

Where's "here"? All I'm asking is, copy the output of the HTML validator, something that UI tests produce in a text file, into the issue. The screenshot lacks a lot of information.

Piedone commented 1 year ago

So, could you please add the necessary information?

Psichorex commented 1 year ago

I will check OC's HTML as soon as I can and everything will be uploaded here.

Psichorex commented 1 year ago

There are validation errors all across OC's admin dashboard. As I told you previously it's not just these 3 on the picture above. Literally every page you open in OC dashboard will produces several and might even different errors.

These 4 screenshots are coming from different routes of the OC admin You can see there are different validation errors all across I can't check every single route it would take ages.

For this to be resolved someone have to go through every page and run this Accessibility Insights for Web and fix these HTML's.

But the most common errors are related to : link-names , aria-command-line, color-contrast, aria-roles, label, button-name But I am sure there are more.

image image image image

Skrypt commented 1 year ago

These are "Accesibility" recommendations; not HTML validation errors.

hishamco commented 1 year ago

As @Skrypt said they are for accessibility, BTW I already started to enhance some of these but I stopped because I'm engaged with many important things. We could file an issue and submit a PR later, for now I suggest to fix HTML validations (as @Piedone said there're 3)

Psichorex commented 1 year ago

HtmlValidationReport.txt Here is the HTML validation report from the UI test. I also copied some of the OC admin dashboard's HTML into a REAL html validator like this: https://validator.w3.org/ And I have seen more errors there than in the UI test's report. This is the /admin image This is /Admin/ContentItems image You can see it found 25 we can subtrack 1-2 because it was missing !DOCTYPE that I didn't copy over but it will still be 20+

Skrypt commented 1 year ago

Some of them seem related to the Admin menu. I may take a look at that part when I get time but not now.

Psichorex commented 1 year ago

Some of them seem related to the Admin menu. I may take a look at that part when I get time but not now.

Literally all of them are in the Admin Dashboard's html as I observed.

Skrypt commented 1 year ago

Yes, I remember that one "number 23 to 25" but I remember that we decided to keep it as it is for now. So, we just need someone that has time to fix all these small issues one by one.

hishamco commented 1 year ago

I might handle them after finishing some PRs