1599935 - "Open With Different Viewer" infobar should be more noticeable

Status: VERIFIED FIXED

(Firefox :: PDF Viewer, enhancement)

Description

[Jason]

Attachment: Forms Screen Shot.png

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:68.0) Gecko/20100101 Firefox/68.0

Steps to reproduce:

Opened a PDF that contains form fields.

Actual results:

The PDF opened with a grey infobar at the top that says "This PDF document contains forms. The filling of form fields is not supported." and a button "Open With Different Viewer".

Expected results:

A yellow or amber infobar (i.e. warning) should appear so more users will notice it. In our organization, we find more users don't notice it, than those who do.

Comment 1

[Jason]

Attachment: widevine screenshot.png

This is a good example of an infobar that is noticeable.

Comment 2

[Mike Kaply [:mkaply]]

This seems like a really good idea. I can do the code, but who would sign off on it?

Comment 3

[Jonas Jenwald [:Snuffleupagus]]

Note that the PDF Viewer notification bar using PRIORITY_INFO_LOW was a conscious change in bug 894576.

Comment 4

[Jason]

(In reply to Jonas Jenwald [:Snuffleupagus] from comment #3)

Note that the PDF Viewer notification bar using PRIORITY_INFO_LOW was a conscious change in bug 894576.

Interesting. There wasn't a lot of conversation about it back then. Generally anyone opening a fillable form would be very happy to be notified by whatever means necessary that there's actually a way to fill it. It's quite similar to the "Enable DRM" notification so that they can actually play the video. When these things don't work, they switch browsers.

I can only speak for my own organization of 2,500 users that I support. For years, we've receive requests regularly about not being able to fill forms. It's not an exaggeration to say it's the most common Firefox-related issue we receive.

Comment 5

[Jason]

(In reply to Jonas Jenwald [:Snuffleupagus] from comment #3)

Note that the PDF Viewer notification bar using PRIORITY_INFO_LOW was a conscious change in bug 894576.

I just noticed there's no mention of forms or fields in this bug. Was the behaviour with Firefox 22 to display a notification for all PDF files perhaps? I could spin up a VM and install to check...

Comment 6

[Mike Kaply [:mkaply]]

It looks like the only reason we display warnings now is for unsupported features. So it does seem like yellow makes more sense:

https://searchfox.org/mozilla-central/rev/073b138dcba41cd3f858522e5f0a9ee73e39afa0/browser/extensions/pdfjs/content/PdfStreamConverter.jsm#467

And yes, that old notification displayed for all PDF files. Basically a "this might not work" before we had real PDF support.

Comment 7

[Mike Kaply [:mkaply]]

Attachment: Bug 1599935 - Make unsupported PDF features a warning. r?bdahl

Comment 8

[Brendan Dahl [:bdahl]]

Stephen,
I think you're the most familiar withe the UX decisions on PDF.js. Do you have any opinion on the color of the fallback bar or know of some who would?

Comment 9

[Stephen Horlander [:shorlander]]

(In reply to Brendan Dahl [:bdahl] (away until 30th) from comment #8)

Stephen,
I think you're the most familiar withe the UX decisions on PDF.js. Do you have any opinion on the color of the fallback bar or know of some who would?

Yes we try and use the more subtle grey infobar for low impact messages.

I think it makes sense to elevate this to the more obvious warning (yellow) status because of the lack of important functionality.

Comment 10

[Mike Kaply [:mkaply]]

bdahl: can you r= https://phabricator.services.mozilla.com/D55527 then?

Thanks!

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7d0cec2c6c40
Make unsupported PDF features a warning. r=bdahl

Comment 12

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/7d0cec2c6c40

Comment 13

[Mike Kaply [:mkaply]]

Comment on attachment 9112984 [details]
Bug 1599935 - Make unsupported PDF features a warning. r?bdahl

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: PDF viewing (and filling) very common in enterprise, and this is a really nice to have.
• User impact if declined: Not obvious PDF form won't work in Firefox
• Fix Landed on Version: 73
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Very low risk, just changes a flag.
• String or UUID changes made by this patch: None

Comment 14

[Cristian Fogel [:cfogel]]

Adding needinfo on :csasca.
Some existing test cases might need review and update after this change.

To be removed after review and verification.

Comment 15

[Catalin Sasca, Desktop QA [:csasca]]

Verified on 73.0b2, and the yellow banner was present for both widevine and PDF's with forms. Also added a test regarding this scenario.

Will verify it too when it gets to esr.

Comment 16

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9112984 [details]
Bug 1599935 - Make unsupported PDF features a warning. r?bdahl

Makes it more obvious when unsupported features are present in a PDF. Approved for 68.5esr.

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr68/rev/737d24fb73b9

Comment 18

[Catalin Sasca, Desktop QA [:csasca]]

Verified on 68.5.0esr (treeherder build) and works as expected.