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

Improve visual accessibility of HTML report #1355

Open
aSemy opened this issue Aug 19, 2022 · 11 comments
Open

Improve visual accessibility of HTML report #1355

aSemy opened this issue Aug 19, 2022 · 11 comments

Comments

@aSemy
Copy link

aSemy commented Aug 19, 2022

Scenario

  • JaCoCo version: 0.8.8
  • Operating system: Windows
  • Tool integration: Maven
  • Description of your use case: I would like to improve the accessibility of the HTML report

Current Behaviour

The JaCoCo HTML report has a static style that does not respect my display preferences. This makes it difficult to use and see the results.

Wanted Behaviour

The JaCoCo HTML report is more accessible so it is easier to read and understand the results.

I would like the HTML report to take advantage of modern CSS features to adjust the visual display based on my preferences.

For example, if I prefer a dark theme, then JaCoCo will show a dark theme. If I prefer high-contrast, then JaCoCo will use a theme with high contrast.

I would like this theme committed to the repository so more experienced people can contribute and make sure that the themes are the best they can be.

Here's a quick demo of prefers-color-scheme: dark

image

And of high contrast prefers-contrast: high

image

Possible Workarounds

In each project run some sort of script that will overwrite the JaCoCo default css and prettify.js?

Proposed tasks

Related issues

Further reading

@marchof
Copy link
Member

marchof commented Aug 19, 2022

Hi @aSemy, thanks for the input! Looks like you bring some expertise here. Would you be able to help us to address these issues?

@aSemy
Copy link
Author

aSemy commented Aug 19, 2022

I'm very flattered because I have no expertise! I'm a very strident backender! :)

I would be very happy to set up the pull requests though, but I'd like for someone else to pick the colours.

@marchof
Copy link
Member

marchof commented Aug 19, 2022

but I'd like for someone else to pick the colours.

Exactly my problem 🤣

So I propose we keep this open until someone with a UI background can help out.

@marchof marchof added help wanted 🖖 We'd like help with this issue component: report labels Aug 19, 2022
@aSemy
Copy link
Author

aSemy commented Aug 19, 2022

There are some preparation steps before then: upgrade prettify.js and refactor the existing style to use CSS vars. Once that's done, it becomes much more easy to set the style for a specific theme on a case-by-case basis.

@thetric
Copy link

thetric commented Dec 18, 2022

The migration to highlight.js itself is simple but brings it own problems. Currently, the HTML for the source view mixes line numbers, code coverage and source code. This is fine in prettify.js but discouraged in hightlight.js:

image

This image shows highlight.js (left) and the current view with prettify.js (right). With highlight.js, neither the line numbers nor the coverage data is visible because its HTML is removed, see https://github.com/highlightjs/highlight.js/wiki/security:

Why is this Bad?

Unescaped HTML can lead to security vulnerabilities in the form of XSS attacks - someone sneaking their HTML (or JavaScript) inside your own - and then doing who knows what manner of mischief.

How can I fix my site to avoid the warning?

Remove all unescaped HTML from all your pre/code blocks.

Can I disable the warning?

Yes, our documentation explains how - but this is not something most users should do - and if there is a legitimate security vulnerability just turning off the warning doesn't resolve the vulnerability.

What if I really, really want to mix real HTML with my highlighted code?

We won't support this in Core. Actual (intentional) HTML inside of code blocks (which is valid in the HTML spec) is simply not something we support because far too easy to shoot yourself in the foot with it. This type of support (for the small number who need it) can easily be added via a plugin. If someone is willing to bring the old HTML merge plugin up-to-date and maintain and support it, they are free to do so. The plugin hook you are looking for is before:highlightElement.

A solution to this problem might be to change the appearance to something like IntelliJ's approach and show the coverage indicator in the gutter:

image

@huangsam
Copy link

huangsam commented Apr 5, 2024

One potential approach to step away from the <pre><span /></pre> approach for line numbers is the way GitHub solves it. By having one div for the line numbers and one div for the code lines. See screenshots below 👇

How it looks from a GitHub web POV:

Screenshot 2024-04-05 at 2 59 17 AM

How the line numbers work from a CSS standpoint:

Screenshot 2024-04-05 at 3 00 50 AM

Spans are currently baked in a HTMLElement construct at the moment, but there's nobody saying that things can be changed up, such that they're no longer used.

@marchof
Copy link
Member

marchof commented Apr 5, 2024

@huangsam Thanks for demonstrating how the line numbers can be extracted from the source. But we still have the tags for coverage highlighting, or is there a similar approach to this?

@huangsam
Copy link

huangsam commented Apr 5, 2024

Yeah then what happens is that we might use non-pre based code lines so that each line is more customized. Imagine an array of <div>s for each code line instead of a <pre> blob with a bunch of lines. I'm not near my laptop, but I'm sure that's what we'd come up with for react-code-lines from the same example above. If we're not prepared to do that radical of a change, then we can always apply the coverage color on the line number itself

@huangsam
Copy link

huangsam commented Apr 5, 2024

Upon a next-level investigation, it looks like GitHub has a <textarea id="read-only-cursor-text-area" ... /> with the raw text and an overlapping <div class="react-code-lines> which has an array of <div>s with <span>s. Here's a screenshot of how it looks like it's implemented.

Text area:

Screenshot 2024-04-05 at 9 32 34 AM

Div lines (comment coloring):

Screenshot 2024-04-05 at 9 36 20 AM

Div lines (Package coloring):

Screenshot 2024-04-05 at 9 39 24 AM

Not super sure why they have both the textarea and the array of div but my original assumption still holds that some <div>-level abstraction exists. Then, they do <span>-based tokenization for each <div> to apply CSS var-based theming.

@marchof
Copy link
Member

marchof commented Apr 5, 2024

Another idea: There is some JavaScript code to apply the syntax highlighting. Can't the same code apply coverage highlighting of we provide coverage info e.g. as JSON?

@huangsam
Copy link

huangsam commented Apr 6, 2024

The original JS code might need to be adjusted if we want to approach things from a div-based approach. I assume it currently applies changes in the pre block

But the idea of having a JSON asset seems to make sense. Although that implies local file access from the browser or a light webserver or an in-memory data structure auto generated in the HTML file itself

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

4 participants