1171799 - bad A11Y/U7Y in PDF viewer toolbar

Status: RESOLVED FIXED

(Firefox :: PDF Viewer, defect, P1)

Description

[Felix Miata]

Attachment: 120 DPI FF38 screenshot in KDE3

Mozilla/5.0 (X11; Linux i686; rv:38.0) Gecko/20100101 Firefox/38.0; Build ID: 20150505103531

Actual behavior:
1-toolbar icons too small
2-toolbar text too small
3-neither icons nor text are affected by zoom level
4-neither icons nor text adapt according to display density or other environmental settings
5-neither icons nor text can be adjusted using default theme options

Expected behavior:
1-Icons and text automatically sized sensibly regardless of display density and environment, like main menu bar and most other UI text.

Comment 1

[Felix Miata]

Attachment: 168 DPI FF38 screenshot in Trinity Desktop Environment r14

Comment 2

[Brendan Dahl [:bdahl]]

We should probably move away from pixels for everything and use device pixels or separate styles for different device-pixel-ratio's.

Comment 3

[Marco Castelluccio [:marco]]

I wonder if this is still a problem.

Comment 4

[Felix Miata]

Attachment: 143 DPI ESR102 screenshot on Trinity Desktop

This screenshot was made using a virgin FF profile.

It has improved, but its text remains arbitrarily smaller than the desktop's UI text, sitting in a sea of whitespace, and its icons remain teensy.

Comment 5

[Anna Yeddi [:ayeddi]]

This is an access-s3 bug per the Accessibility triage guidelines.

Touch/click target size is recommended to be at least 44 x 44 CSS px and to provide sufficient whitespace to avoid clicking a neighbor instead of the target. More on touch target sizes on desktop and mobile can be found in the BBC A11y Guide and in the WCAG 2.2 Success Criterion 2.5.5 Target Size (Enhanced) and Success Criterion 2.5.8 Target Size (Minimum). Also, it is expected that the font of the PDF viewer UI would follow the browser UI and allow to be resized by the user (vaguely refer to SC 1.4.4 Resize Text)

Comment 6

[BMO Github Automation]

Attachment: [mozilla/pdf.js] Refactor the toolbar html & css to improve its overall accessibility (bug 1171799, bug 1855695) (#18385)

Comment 7

[BMO Github Automation]

Attachment: [mozilla/pdf.js] Allow to change the toolbar height when changing the pref toolbar.density in Firefox (bug 1171799) (#18397)

Comment 8

[Calixte Denizet (:calixte)]

Attachment: Bug 1171799 - Dispatch an event in pdf.js when the pref toolbar.density is changed r=#pdfjs-reviewers

The goal is to be able to change the pdf.js toolbar height depending on the toolbar.density value.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c6dd3ed97864
Dispatch an event in pdf.js when the pref toolbar.density is changed r=pdfjs-reviewers,marco

Comment 10

[Cristian Tuns]

Backed out for causing wpt failures in plugin-document.historical.html

• Backout link
• Push with failures
• Failure Log
• Failure line:TEST-UNEXPECTED-TIMEOUT | /html/browsers/browsing-the-web/navigating-across-documents/plugin-document.historical.html | expected OK

Comment 11

[Cristian Tuns]

We also have other failures:
geckoview: https://treeherder.mozilla.org/logviewer?job_id=465655758&repo=autoland
Mochitest: TEST-UNEXPECTED-FAIL | toolkit/components/pdfjs/test/test_pdf_file_in_iframe.html | Test timed out. -
TEST-UNEXPECTED-FAIL | toolkit/components/pdfjs/test/test_pdf_file_in_iframe.html | [SimpleTest.finish()] No checks actually run. (You need to call ok(), is(), or similar functions at least once. Make sure you use SimpleTest.waitForExplicitFinish() if you need it.)
Possibly this: https://treeherder.mozilla.org/logviewer?job_id=465656835&repo=autoland
Wpt 21: https://treeherder.mozilla.org/logviewer?job_id=465657295&repo=autoland
Fenix: https://treeherder.mozilla.org/logviewer?job_id=465664017&repo=autoland , https://treeherder.mozilla.org/logviewer?job_id=465663929&repo=autoland

I assume all these failures on Android 7.0: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=70c615f711f3c4b76a1863df79f1a7eda6425e68&searchStr=android%2C7.0&selectedTaskRun=AgwBQ1oKQmKs5EVutaq0JA.0

Comment 12

[Aaron Train [:aaronmt]]

Hello, we have a number of UI tests PDF tests that started failing on Fenix also prior to back-out (as part of ui-test-apk jobs on the push). They can be scheduled via ./mach try --preset firefox-android as well ran locally when you have Fenix up and running: https://firefox-source-docs.mozilla.org/mobile/android/fenix/UI-Tests.html#running-ui-tests

Comment 13

[Aaron Train [:aaronmt]]

Attachment: Screenshot 2024-07-09 at 5.44.59 PM.png

In the case for Fenix, all attempts at loading a PDF were stuck and hung (as seen on devices and emulators on Firebase Test Lab).

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d98e5190542e
Dispatch an event in pdf.js when the pref toolbar.density is changed r=pdfjs-reviewers,marco

Comment 15

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/d98e5190542e

Comment 16

[BMO Github Automation]

Attachment: [mozilla/pdf.js] Introduce a CSS variable for the toolbar height (bug 1171799) (#18518)

Comment 17

[Mathew Hodson]

Can this be closed now that pdf.js was updated in bug 1911184?

Comment 18

[Marco Castelluccio [:marco]]

Not yet, there is still something left to do here: the patch in this bug (which was backed-out) and https://github.com/mozilla/pdf.js/pull/18385.

Comment 19

[BMO Github Automation]

Attachment: [mozilla/pdf.js] Increase the size of the toolbar depending on the uidensity (bug 1171799) (#18786)

Comment 20

[BugBot [:suhaib / :marco/ :calixte]]

Since nightly and release are affected, beta will likely be affected too.
For more information, please visit BugBot documentation.

Comment 21

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:calixte, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval. Also, don't forget to request an uplift for the patches in the regression caused by this fix.
• If no, please set status-firefox131 to wontfix.

For more information, please visit BugBot documentation.

Comment 22

[Marco Castelluccio [:marco]]

The toolbar refactoring is too risky to consider it for an uplift.