Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Call DownloadHandler::CanDownload when downloading a PDF file from the PDF viewer #3783

Open
lucamuscat opened this issue Sep 4, 2024 · 1 comment
Labels
enhancement Enhancement request

Comments

@lucamuscat
Copy link

lucamuscat commented Sep 4, 2024

Describe the bug
Typically, DownloadHandler::CanDownload is called before any download, giving the developer the opportunity to decide whether a file should be downloaded. DownloadHandler::CanDownload is not executed when a PDF file is downloaded through the PDF viewer plugin, circumventing the logic found in DownloadHandler::CanDownload.

Although one may argue that the PDF is technically already downloaded, thereby making this method call redundant, it subverts the exceptation that DownloadHandler::CanDownload method is called on all downloads.

My current workaround is to move my DownloadHandler::CanDownload logic into DownloadHandler::OnBeforeDownload, and returning false in DownloadHandler::OnBeforeDownload, allowing the alloy runtime to cancel the download by default. This is a hacky/dirty workaround which I wish to avoid.

To Reproduce
Steps to reproduce the behavior:

  1. Clone my fork of cef-project containing the MVP of the bug
  2. Build the project, and run the minimal executable. DownloadHandler::CanDownload will log a message starting with CanDownload when triggered, allowing you to grep for the log.
  3. Download a normal file as a sanity check, and observe the CanDownload log
  4. Open a PDF file, and press the download button, observe that the log is not emitted.

Expected behavior
DownloadHandler::CanDownload is called when downloading a PDF file through the PDF viewer.

Versions (please complete the following information):

  • OS: Ubuntu 20.04.1
  • CEF Version: 127.3.4+ge9e2e14+chromium-127.0.6533.100_linux64

Additional context
So far, I have only tried reproducing the bug in the minimal project in cef-project, but I also observe this behaviour in another project.

Let me know if I can provide more information. If you know where I can look to start fixing this bug, kindly let me know.

Regards,
Luca

@magreenblatt
Copy link
Collaborator

magreenblatt commented Sep 25, 2024

As background, the "Download" button in the PDF viewer should trigger a call to PdfViewWebPlugin::HandleSaveMessage (via onDownloadClick_ in the JS code). There is a different code path for original vs edited PDF files (SaveToFile vs SaveToBuffer). SaveToFile calls PDFDocumentHelper::SaveUrlAs and SaveToBuffer calls saveData_ (JS code). The SaveToFile case should call DownloadManagerImpl::DownloadUrl. However, the SaveToBuffer case would not trigger a download because it operates using local (in memory) PDF modifications.

@magreenblatt magreenblatt added enhancement Enhancement request and removed bug Bug report labels Sep 25, 2024
@magreenblatt magreenblatt changed the title DownloadHandler::CanDownload does not run when downloading a PDF file from the PDF viewer Call DownloadHandler::CanDownload when downloading a PDF file from the PDF viewer Sep 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhancement request
Projects
None yet
Development

No branches or pull requests

2 participants