Closed Bug 1777257 Opened 2 years ago Closed 2 years ago

Accessibility review for PDF editing

Categories

(Firefox :: PDF Viewer, task)

task

Tracking

()

RESOLVED FIXED
a11y-review passed

People

(Reporter: marco, Assigned: Jamie)

References

(Depends on 1 open bug)

Details

We have already asked for feedback to James and Morgan, so we are already aware of some accessibility issues and we have already fixed some.

Description:
We are adding support for basic editing of PDFs.

How do we test this?
Open any PDF, use the buttons from the toolbar to try and edit them.

When will this ship? Firefox 104 (tentatively)
Tracking bug/issue: bug 1774350
Design documents (e.g. Product Requirements Document, UI spec): N/A
Engineering lead: Calixte Denizet

The accessibility team has developed the Mozilla Accessibility Release Guidelines which outline what is needed to make user interfaces accessible:
https://wiki.mozilla.org/Accessibility/Guidelines
Please describe the accessibility guidelines you considered and what steps you've taken to address them:
We are taking them into account while working on the feature.

Describe any areas of concern to which you want the accessibility team to give special attention:
Toolbar buttons
Interactions with the page to draw/insert text

See Also: → 1776906

[Removed unintentional comment.]

Assignee: nobody → jteh
Status: NEW → ASSIGNED

Thanks for filing this.

I provided some review in https://github.com/mozilla/pdf.js/pull/15110. Morgan also verified things with HCM.

Following are some other issues which I'm not sure should be covered there or elsewhere:

  1. It's currently difficult for a screen reader user to enable the editor. Normally, a screen reader user activates controls using the accessibility API. Gecko dispatches such an activation as a click, but with this annotation editor, this doesn't work. This is almost certainly to do with the various layers involved here (canvas, text, etc.). This is further complicated by the use of aria-owns, which causes the a11y tree to diverge from the DOM tree, and events bubble via the DOM tree. This really should be a blocker, but on the other hand, I'm not entirely sure how we can fix this.
    How do clicks for the annotation editor get handled? Is there any way we could have a click handler in the text layer which handles things correctly?

  2. I've learned that the editor can be dismissed, but there doesn't appear to be a way to dismiss it from the keyboard. Perhaps control+enter or escape could do this? While redundant, I'd suggest both just because some users might be less familiar with control+enter, even though it's probably more "correct" than escape. This is a blocker if it's necessary to dismiss the editor in order for the annotations to be saved, but probably not if it isn't, unless there's something else the user can't do without disabling the editor.

  3. Because I can't dismiss the editor, I can't verify this, but we need to verify how the editor is rendered when it is disabled. I'm assuming that visually, you can see there is an annotation. If it's just text, this is not going to be obvious to a screen reader user. Perhaps we could use role="insertion" to mark such text? This probably isn't a blocker because the user is hopefully aware of the text they entered.

  4. I don't seem to be able to see the annotations when I save and reload the PDF. Calixte noted on the pull request:

    Text annotation are rendered as basic FreeText annotations it appears that I didn't think about a11y when saving: I need to check how it works in pdf format.

    This needs to be checked and potentially fixed. If it's broken, this would be a blocker.
    If there is a visual indication that these are annotations, there should be an a11y indication also; e.g. role="insertion".
    Also, are the annotations in this state the same as a disabled editor (3) or is this different?

Depends on: 1780375

The suggestion mentioned in point 2 was implemented in https://github.com/mozilla/pdf.js/pull/15186.

Calixte filed bug 1780375 for point 4.

Depends on: 1786339

Thanks Jamie and Morgan for your review! We have fixed all points you found, except 1 which remains open but is technically super hard to implement. I've filed a separate bug (bug 1786339) to track that and will close this bug as we can consider the accessibility review to be finished.

Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED

Marking the a11y review as passed as we're okay with this shipping, noting that we should ideally find a way to solve bug 1786339 for the best user experience.

a11y-review: requested → passed
You need to log in before you can comment on or make changes to this bug.