kolibri icon indicating copy to clipboard operation
kolibri copied to clipboard

[PDF_Viewer] Adding struct tree layer

Open AlexVelezLl opened this issue 3 years ago • 5 comments

Summary

Adding a new layer that adds a PDF tree structure of tagged PDFs so that screen readers can notice the semantic structure of a tagged PDF.

With this layer screen readers should now be able to recognize among other things:

  • Headings of different levels.
  • Paragraphs
  • Lists/List items
  • Tables
  • Figures alt text
  • Links

References

This PR solves the lack of semantic structure of the text layer that was missing in #9536.

Reviewer guidance

  1. Go to learn tab
  2. pick any reading-type resource that has a tagged PDF (e.g. The pdfs that are in the Kolibri QA channel)
  3. Use a screen reader and note if the screen reader is able to identify the semantic structure of the pdf.

Testing checklist

  • [x] Contributor has fully tested the PR manually
  • [ ] If there are any front-end changes, before/after screenshots are included
  • [ ] Critical user journeys are covered by Gherkin stories
  • [ ] Critical and brittle code paths are covered by unit tests

PR process

  • [x] PR has the correct target branch and milestone
  • [ ] PR has 'needs review' or 'work-in-progress' label
  • [ ] If PR is ready for review, a reviewer has been added. (Don't use 'Assignees')
  • [ ] If this is an important user-facing change, PR or related issue has a 'changelog' label
  • [ ] If this includes an internal dependency change, a link to the diff is provided

Reviewer checklist

  • Automated test coverage is satisfactory
  • PR is fully functional
  • PR has been tested for accessibility regressions
  • External dependency files were updated if necessary (yarn and pip)
  • Documentation is updated
  • Contributor is in AUTHORS.md

AlexVelezLl avatar Aug 01 '22 20:08 AlexVelezLl

Alex, excellent progress, you're spoiling us! 👏🏽 👏🏽 👏🏽

Headings, lists, links, even tables, all well recognized and announced by the screen reader 🎉

The only sore point are the images that are not announced, nor their alt text is read out, even though it's there as an aria-label. I don't think it's because of your implementation, they do not seem to be recognized and announced by NVDA even if I open the PDF directly in the browser. Could it be because the role=figure is used? Would it be viable to try role=img instead?

Win10 (start)  Running  - Oracle VM VirtualBox_001

radinamatic avatar Aug 03 '22 05:08 radinamatic

Another point for potential improvement would be to explore the possibility to exclude empty paragraphs from the text layer as those are announced as blank, blank, blank by NVDA, and it's an annoyance.

Win10 (start)  Running  - Oracle VM VirtualBox_002

Sighted PDF users will not miss them when copying text, they can't be highlighted, and are just a bad experience for blind readers.

Empty paragraphs in the Acrobat tag tree view:

2022-08-03_8-15-49

radinamatic avatar Aug 03 '22 06:08 radinamatic

This is a thing of beauty, now Kolibri PDF viewer offers a slightly better experience for screen reader users compared even to the native browser PDF reader on this aspect... 🤩 😍 Thank you, @AlexVelezLl!!!

Since you've been churning these improvements with such efficiency, here is one last request: let's try to implement a similar rendering of the PDF file pages as pdf.js does.

You implemented the rendering of the PDF page as a section HTML element with a pdf-page class, but class names are not semantic and are not accessible to assistive technology. On another hand pdf.js renders a page as div element with a role="region" and aria-label="Page N" which are both revealed to screen readers, and could potentially help navigate the PDF document more easily.

Kolibri pdf.js
LEDev2202 (start)  Running  - Oracle VM VirtualBox_002 LEDev2202 (start)  Running  - Oracle VM VirtualBox_003

My thinking goes in direction to try to offer a similar experience and path to knowing the PDF page numbers in Kolibri viewer as in the browser PDF reader, for those users who are already familiar and use the latter.

radinamatic avatar Aug 09 '22 23:08 radinamatic

@radinamatic It sounds great! I had not noticed that this was missing, I already pushed those changes, although the translations are missing.

AlexVelezLl avatar Aug 10 '22 14:08 AlexVelezLl

The new page numbering works well, and I like it 💯 , the only twist is that the lazy loading is messing with the order when one scrolls up and down the document 😳 I'm guessing there isn't really much we can do about it, right?

LEDev (start)  Running  - Oracle VM VirtualBox_003

radinamatic avatar Aug 17 '22 10:08 radinamatic

@radinamatic I think it's a bit complicated because all of that is handled by the RecycleList internally. Although I could search if there is any configuration for that 🤔

AlexVelezLl avatar Aug 17 '22 15:08 AlexVelezLl

As discussed in our weekly call, this is working great, and ready to merge!

Thank you, I am so looking forward to what's coming next! 😍

radinamatic avatar Aug 17 '22 20:08 radinamatic