Skip to content

Conversation

@jooyoungseo
Copy link

@jooyoungseo jooyoungseo commented Jun 2, 2020

This PR addresses #1828, and have been tested and confirmed with many hours of testing with various combinations of different screen readers and browsers.

Current Issue

When code highlight is used for html output, screen reader users hear unnecessary and annoying information in codeblock, such as "#cb1-1," "#cb3-1," "#cb5-1," and "#cb5-2."

This is because the code anchor (<a>) produced by pandoc skyligting has empty content, and each id attribute value is read out by screen readers.

To avoid this issue, we need to put aria-hidden = "true" per each empty <a> code anchor within "source code" class.

While the pandoc fix will be already made and will be available in next version see pandoc/issue#6352, we need a backword solution towards current and previous pandoc versions (between v2.7.3 and v2.9.2.1).

I have written a simple js that can address this issue, and this simple fix can dramatically improve the screen reader accessibility for any html_document-like output.

Testing File


---
output:
  html_document:
    highlight: "tango"
---

```{r}
summary(airquality$Ozone)
mean(airquality$Ozone)
```


```{r}
data(iris)
tail(iris)
```

Output to Screen Readers (without this fix)

  • Notes: this issue can be resolved with this PR.
#cb1-1
 summary(airquality$Ozone)
## Min. 1st Qu. Median Mean 3rd Qu. Max. NA’s 
## 1.00 18.00 31.50 42.13 63.25 168.00 37
#cb3-1
 mean(airquality$Ozone)
## [1] NA
#cb5-1
 data(iris)
#cb5-2
 tail(iris)
## Sepal.Length Sepal.Width Petal.Length Petal.Width Species
## 145 6.7 3.3 5.7 2.5 virginica
## 146 6.7 3.0 5.2 2.3 virginica
## 147 6.3 2.5 5.0 1.9 virginica
## 148 6.5 3.0 5.2 2.0 virginica
## 149 6.2 3.4 5.4 2.3 virginica
## 150 5.9 3.0 5.1 1.8 virginica

@jooyoungseo
Copy link
Author

@atusy, it still does not work. Haha, I do not understand why this simple thing takes so long time. Any advice would be greatly appreciated.

@jooyoungseo
Copy link
Author

FYI, the js itself works perfectly when I render with includes(after_body()).

@jooyoungseo
Copy link
Author

jooyoungseo commented Jun 3, 2020

@atusy, thanks very much! However, here is a very picky issue.

I have tested and confirmed that neither document.addEventListener('DOMContentLoaded', function() { ... }); nor $(document).ready(function() { ... }); adds the aria-hidden attribute to the empty anchors.

This is very weird because it works successfully if I move the script down after body (of course, after removing the dom load dispatch).

Humm... I think I need to dig into how to tackle this tacky problem.

@atusy
Copy link
Collaborator

atusy commented Jun 3, 2020

@jooyoungseo

What do you mean by the empty anchors?
Would you give me a reproducible example and an expected result?
My example below works well.

---
title: "Untitled"
output: 
  html_document:
    mathjax: null
    theme: null
    highlight: pygments
---

```r
iris
```

I see following result with the inspector.

<a href="#cb1-1" aria-hidden="true"></a>

@jooyoungseo
Copy link
Author

jooyoungseo commented Jun 3, 2020

@atusy, I am so excited to tell you that we have made it! Please kindly ignore the previous comment (it turned out the issue caused by a screen reader bug delay caught by the previous buffer history). With several testing with various combinations of browsers and screen readers, this fix is now working welll and safely! :) Thanks very much again. I will add a line to NEWS.md shortly.

@jooyoungseo jooyoungseo changed the title Draft addressing #1828 Screen reader accessibility improvement for highlighted codeblock of html output Jun 3, 2020
@atusy
Copy link
Collaborator

atusy commented Jun 3, 2020

Congrats and thank you for crediting me!!

Copy link
Contributor

@yihui yihui left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. I'll make some cosmetic changes and merge the PR. Thank you!

…his function in the (far) future, so do not export this function yet, unless other package authors ask for it
@yihui yihui merged commit b133ce4 into rstudio:master Jun 18, 2020
jonathan-g added a commit to jonathan-g/rmarkdown that referenced this pull request Jul 7, 2020
* rstudio/master: (61 commits)
  make metadata available before running pre_knit (rstudio#1855)
  fix rstudio#1815: apply the lua filters pagebreak.lua and latex-div.lua to beamer_presentation
  require the data-latex attribute on fenced Div's again: rstudio#1779 (comment)
  start the next version
  CRAN release v2.3
  fold any code blocks with the class `foldable` for html_document output (rstudio#1835)
  fix rstudio#1828: add aria-hidden = "true" to empty <a> tags in highlighted code blocks generated by Pandoc to improve screen reader accessibility (rstudio#1833)
  add a news item for rstudio#1832
  roxygenize
  no need to turn on --file-scope or actually write the split content into files if the split content is of length < 2
  eliminate renumber_footnotes option
  only test two pandoc versions (devel and RStudio version) and upgrade default to 2.7.3 (rstudio#1846)
  close rstudio#1838: test more R versions on Travis (rstudio#1845)
  renumber_footnotes output format option
  add the <div class="kable-table"> only when the output format is HTML, otherwise the div will be converted to a LaTeX environment, leading to the bug https://stackoverflow.com/q/62340425/559676
  change name of file_scope argument to references_scope
  re-roxygenize
  Add `publish_site()` function for "one-button" publishing of R Markdown websites.
  Enable use of pandoc --file-scope for input files originating from multiple Rmds (rstudio#1837)
  Added missing lang attribute to ioslides_presentation template (rstudio#1841)
  ...
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 15, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants