1655525 - Using <embed> for PDFs does not result in a download if PDF.js is disabled

Status: ASSIGNED

(Firefox :: PDF Viewer, defect, P2)

Description

[Kevin Brosnan [Ex-Mozilla]]

Steps to reproduce

1. Use Fenix or set pdfjs.disabled to true in Desktop
2. Go to http://plantuml.com/fr/guide

Expected behavior

Show an actionable download

Actual behavior

blank page

Device information

• Android device: Android 10 Pixel 2
• Fenix version: Firefox 81

Note

webcompat issue: https://github.com/webcompat/web-bugs/issues/55689

<div id="root">
  <embed src="http://pdf.plantuml.net/PlantUML_Language_Reference_Guide_en.pdf" type="application/pdf" width="100%" height="100%">
</div>

On Chrome a download button is displayed.

Comment 1

[Karl Dubost💡 :karlcow]

Attachment: self contained pdf-js

online version A test case https://www.la-grange.net/2020/07/28/moz/embed-pdf.html

• Desktop
  • firefox Nightly OK
  • chrome Canary OK
• Mobile
  • Firefox preview FAIL
  • Chrome Canary OK

Comment 2

[Hsin-Yi Tsai (she/her) [:hsinyi]]

Trying to move to a better component...

Comment 3

[Kevin Brosnan [Ex-Mozilla]]

Not sure how this is PDF reader? It happens without the PDF Viewer code.

Comment 4

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

The severity field is not set for this bug.
:bdahl, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 5

[:Gijs (he/him)]

Kevin, this looks like a dupe of bug 1500404, maybe (setting pdfjs.disabled will have roughly the same effect as changing the default action for PDFs)? Is this a regression compared to Fennec behaviour?

Comment 6

[Kevin Brosnan [Ex-Mozilla]]

Fennec has the same behavior. This seems slightly different in that we don't get a prompt to download the file on Android and functionally that is a ton worse. Maybe this should live in Gekcoview until they have a chance to investigate?

Comment 7

[:Gijs (he/him)]

I believe the relevant code is https://searchfox.org/mozilla-central/rev/eb9d5c97927aea75f0c8e38bbc5b5d288099e687/dom/base/nsContentUtils.cpp#9629,9639-9643 and its callers, where we'll deal with the content via pdf.js if available, but won't if it's not.

It's not clear to me what the "right" fix would be - I don't see an obvious way to cause the load within the object/embed to trip a download instead - if we pretend to support the mimetype when we do not, I think other code could be upset (cf. https://searchfox.org/mozilla-central/rev/eb9d5c97927aea75f0c8e38bbc5b5d288099e687/dom/base/nsObjectLoadingContent.cpp#988-990 ). If we want to display fallback content for this case if the website hasn't provided any, I think that might be the most straightforward option? AIUI we could probably do it with a UA widget that uses a closed shadow root that displays a button that downloads the PDF. Unsure how web-compatible that is... but maybe better for usability than doing nothing?

But I'm not a DOM expert, so I'll move this along and hope the good people who know that code better can chime in. :-)

Comment 9

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

The severity field for this bug is relatively low, S3. However, the bug has 5 See Also bugs.
:hsinyi, could you consider increasing the bug severity?

For more information, please visit auto_nag documentation.

Comment 10

[Hsin-Yi Tsai (she/her) [:hsinyi]]

Would be nice to get a view for priority from web compat team. (Oh, karl cleared a previous webcompat-Priority? request but I am not sure I know why)

Comment 11

[Hsin-Yi Tsai (she/her) [:hsinyi]]

Emilio, are you the right person to ask for more thoughts or suggestions for comment 6? Or maybe Kris?

Comment 12

[Kevin Brosnan [Ex-Mozilla]]

On mobile any user that encounters a page that contains this cannot access the PDF at all. We don't ship with PDF.js. The download button suggestion from Gijs seems like a path to investigate.

Comment 13

[Hsin-Yi Tsai (she/her) [:hsinyi]]

(In reply to Hsin-Yi Tsai [:hsinyi] from comment #11)

Emilio, are you the right person to ask for more thoughts or suggestions for comment 6? Or maybe Kris?

Sorry, meant to be comment 7 :)

Comment 14

[Hsin-Yi Tsai (she/her) [:hsinyi]]

Thanks Andreas!

Comment 15

[Hsin-Yi Tsai (she/her) [:hsinyi]]

Bumping severity & priority as it breaks the pdf experience on Android.

Comment 16

[Kris Maglione [:kmag]]

I think that displaying a button to download the PDF, possibly in a similar manner to the way we load the PDF.js UI, is probably our best bet. We typically wouldn't want to automatically display a download prompt for content loaded in an <object> or <embed> since they're explicitly asking to be embedded.

Comment 17

[Andreas Farre [:farre]]

Attachment: WIP: Bug 1655525 - Create fallback when pdfjs isn't enabled.

Enabled behind the pref: pdfjs.fallback.

Also requires pdfjs.disabled to be true.

Comment 18

[Andreas Farre [:farre]]

I took a stab at it, but it is horrible. It works, but it's horrible. Some obvious problems:

• <object> has a mechanism for fallback, but displaying its children on failure, but <embed> doesn't. This patch avoids adding a download button if the element is <object> and has fallback. But, this is fairly arbitrary and should probably be thought about a bit more.

• No localization.

• No styling.

Hsin-Yi, should we see if someone else wants to work at finishing this while I'm away?

Comment 19

[Hsin-Yi Tsai (she/her) [:hsinyi]]

Hi Sean, do you want to give it a try? (In reply to Andreas Farre [:farre] from comment #18)

I took a stab at it, but it is horrible. It works, but it's horrible. Some obvious problems:

• <object> has a mechanism for fallback, but displaying its children on failure, but <embed> doesn't. This patch avoids adding a download button if the element is <object> and has fallback. But, this is fairly arbitrary and should probably be thought about a bit more.

• No localization.

• No styling.

Hsin-Yi, should we see if someone else wants to work at finishing this while I'm away?

Hi Sean, do you want to give it a try?

Comment 20

[Sean Feng [:sefeng]]

yeah, sure, trying

Comment 21

[Sean Feng [:sefeng]]

Attachment: how_it_looks_in_chrome.png

I attached a picture of what it looks like on Chrome Android when they can't render the PDF.

Comment 22

[Sean Feng [:sefeng]]

Attachment: pdf_chrome_desktop.png

This is what it looks like in Chrome desktop.

Comment 23

[Nicole Weber [:topotropic]]

Sean, let me know if there is anything you need from UX. Thanks!

Comment 24

[Sean Feng [:sefeng]]

Topotropic and I chatted about this offline and we decided to just go with a simple Open PDF button.

Comment 25

[Marco Castelluccio [:marco]]

Is this still relevant now that we ship PDF.js on Android? If it is only applicable when PDF.js is disabled, we should decrease the severity from S2 since it's not a "normal" configuration.

Comment 26

[:Gijs (he/him)]

(In reply to Marco Castelluccio [:marco] from comment #25)

Is this still relevant now that we ship PDF.js on Android? If it is only applicable when PDF.js is disabled, we should decrease the severity from S2 since it's not a "normal" configuration.

I think that configuration is quite common for enterprise though, and it's unfortunate that in that case there is just no clue to the user that PDFs are present, when that does "just work" in other browsers... Maybe mkaply can comment on how common it is for enterprise users to turn off PDF.js - I just know there's a policy for it.

Comment 27

[Mike Kaply [:mkaply]]

Yes, enterprises disable PDF.js because in many cases they need/have to use Adobe (especially given things like not honoring document security by default)

So this is still relevant.

Comment 28

[Sean Feng [:sefeng]]

Calixte and I chatted about this, and Calixte will tackle this!

Comment 29

[Tiffanie Shakespeare [:tif]]

Can someone help me understand how this relates to the other work that is happening with regards to PDFs in Fenix that Verdi is supporting?

Comment 30

[Marco Castelluccio [:marco]]

(In reply to Tiffanie Shakespeare [:tif] from comment #29)

Can someone help me understand how this relates to the other work that is happening with regards to PDFs in Fenix that Verdi is supporting?

With the integration of the PDF viewer in Fenix, this bug can be considered a Desktop-only bug.

Comment 31

[Tiffanie Shakespeare [:tif]]

Thanks Marco!