1838883 - pdf.js missing search bar icons

Status: VERIFIED FIXED

(Firefox :: PDF Viewer, defect)

Description

[Chih-Hsuan Yen [:yan12125]]

Attachment: search-missing-icon.png

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:109.0) Gecko/20100101 Firefox/114.0

Steps to reproduce:

Open a random PDF and search something with Ctrl+F

Actual results:

Icons for "Previous" and "Next" buttons are missing

Expected results:

Those buttons have icons

Comment 1

[Chih-Hsuan Yen [:yan12125]]

Preliminary investigation:

From developer tools, the icon URLs are resource://pdf.js/web/images/findbarButton-previous.svg and resource://pdf.js/web/images/findbarButton-next.svg, while those two files are missing - unzip -l /usr/lib/firefox/omni.ja | grep findbar- shows nothing.

I noticed this issue on Firefox 114 provided by Arch Linux, and the issue also happens for the latest nightly version (tested the version downloaded on 2023-06-16). On the other hand, search bar icons are fine on https://mozilla.github.io/pdf.js/web/viewer.html, so this is probably an issue in how Firefox integrates pdf.js.

Comment 2

[BugBot [:suhaib / :marco]]

The Bugbug bot thinks this bug should belong to the 'Firefox::PDF Viewer' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Comment 3

[Calixte Denizet (:calixte)]

Arrf... stupid typo error:
https://searchfox.org/mozilla-central/source/toolkit/components/pdfjs/jar.mn#34

Comment 4

[Calixte Denizet (:calixte)]

:standard8, could we lint this error, e.g. if there are no files with a given pattern then warns/throws ?

Comment 5

[Jonas Jenwald [:Snuffleupagus]]

Attachment: Bug 1838883 - Correctly bundle the PDF Viewer findbar icons. r=#pdfjs-reviewers

Because of a typo, the findbar icons weren't included in the build.

The reason that this has gone unnoticed until now is that the Firefox PDF Viewer uses the browser findbar by default, and the affected findbar is only needed for PDF documents embedded in e.g. <iframe> elements; this can be reproduced with e.g. https://web.archive.org/web/20220309040754/https://www.adobe.com/content/dam/acom/en/devnet/pdf/pdfs/PDF32000_2008.pdf

Comment 6

[Mark Banner (:standard8)]

(In reply to Calixte Denizet (:calixte) from comment #4)

:standard8, could we lint this error, e.g. if there are no files with a given pattern then warns/throws ?

We already have browser_all_files_referenced.js but that is currently excluding the image files: https://searchfox.org/mozilla-central/rev/986024d59bff59819a3ed2f7c1d0f5254cdc3f3d/browser/base/content/test/static/browser_all_files_referenced.js#41-42

Presumably because of using the wildcards in the jar.mn.

Comment 7

[BugBot [:suhaib / :marco]]

Set release status flags based on info from the regressing bug 1808197

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/021a7eb3c5e1
Correctly bundle the PDF Viewer findbar icons. r=pdfjs-reviewers,calixte

Comment 9

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/021a7eb3c5e1

Comment 10

[BugBot [:suhaib / :marco]]

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

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox115 to wontfix.

For more information, please visit BugBot documentation.

Comment 11

[Calixte Denizet (:calixte)]

Comment on attachment 9339585 [details]
Bug 1838883 - Correctly bundle the PDF Viewer findbar icons. r=#pdfjs-reviewers

Beta/Release Uplift Approval Request

• User impact if declined: The users searching for a string in a pdf embedded in an iframe won't have a good experience because some icons are missing.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: - open https://pspdfkit.com/blog/2021/how-to-embed-a-pdf-viewer-in-your-website/#_1-using-the-embed-tag

• click on the magnifying glass
• check that the left and right chevrons icons are here

• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): It's just a matter of embedding two svg files in omni.ja
• String changes made/needed:
• Is Android affected?: No

Comment 12

[Chih-Hsuan Yen [:yan12125]]

Thank you very much for the fast fix! I can confirm the issue is fixed on 116.0a1（2023-06-19）.

Has the fix been verified in Nightly?: No

Maybe this can be changed to yes?

Comment 13

[Chih-Hsuan Yen [:yan12125]]

Attachment: Fixed find bar on 116.0a1（2023-06-19）

Comment 14

[Donal Meehan [:dmeehan]]

Comment on attachment 9339585 [details]
Bug 1838883 - Correctly bundle the PDF Viewer findbar icons. r=#pdfjs-reviewers

Approved for 115.0b8.

Comment 15

[Donal Meehan [:dmeehan]]

https://hg.mozilla.org/releases/mozilla-beta/rev/625a1960a374

Comment 16

[Chih-Hsuan Yen [:yan12125]]

Probably revising the Beta/Release Uplift Approval Request is not needed as it's already approved

Comment 17

[Ardelean Oana, Desktop Test Engineering [:oardelean]]

Reproducible on a 2023-06-15 Nightly build on Windows 10.
Verified as fixed on Firefox 115.0b8(build ID: 20230620180051) and Nightly 116.0a1(build ID: 20230620212415) on Windows 10, macOS 12, Ubuntu 22.
‘Previous’ and ‘Next’ buttons are now visible and working as intended.