Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Clicking through PDF link doesn't open the file #12381

Closed
srirambv opened this issue Dec 22, 2017 · 5 comments · Fixed by brave/muon#412
Closed

Clicking through PDF link doesn't open the file #12381

srirambv opened this issue Dec 22, 2017 · 5 comments · Fixed by brave/muon#412

Comments

@srirambv
Copy link
Collaborator

srirambv commented Dec 22, 2017

Test plan

  1. Clean install (clear profile, etc)
  2. Search for bitcoin pdf
  3. Click on the first link; it should load

Description

Clicking through PDF link doesn't open the file

Steps to Reproduce

  1. Clean install 0.20.9
  2. Search for bitcoin pdf
  3. Click on the first link, doesn't open the pdf
  4. Ctrl+Click on the link, pdf opens in a new tab

Actual result:
Clicking through PDF link doesn't open the file

Expected result:
Clicking through PDF link should open the file

Reproduces how often:
100%

Brave Version

about:brave info:

Brave 0.20.9
V8 6.3.292.48
rev b5aec10
Muon 4.5.31
OS Release 4.13.0-21-generic
Update Channel Beta
OS Architecture x64
OS Platform Linux
Node.js 7.9.0
Brave Sync v1.4.2
libchromiumcontent 63.0.3239.108

Reproducible on current live release:
No

Additional Information

On Linux/Windows it doesn't click through
On macOS, it prompts to download the file when trying to click through the link. Confirmed by @LaurenWags

@srirambv srirambv added 0.20.x issue first seen in 0.20.x plugin/pdfjs regression labels Dec 22, 2017
@srirambv srirambv added this to the 0.20.x (Beta Channel) milestone Dec 22, 2017
@petemill petemill self-assigned this Dec 22, 2017
@petemill
Copy link
Member

Looks like the browser-laptop pdfJS.js module correctly identifies that a PDF file should be loaded from the extension, but does not know which tabId to load the pdf file in to, since muon is not providing a tabId via the following in app/filtering.js (line 355)

function registerForHeadersReceived (session, partition) {
  const isPrivate = module.exports.isPrivate(partition)
  session.webRequest.onHeadersReceived(function (details, muonCb) {
...

details.tabId, which is passed to pdfJS, which then calls appActions.loadURL with the pdf viewer location, is -1

@bsclifton
Copy link
Member

bsclifton commented Jan 4, 2018

@kjozwiak @LaurenWags this is a weird one.

Please do feel free to try the steps above and make sure it works... but if the issue isn't present in existing 0.19.x release, perhaps we should move this back to 0.20.x

@kjozwiak
Copy link
Member

kjozwiak commented Jan 4, 2018

@bsclifton I couldn't reproduce this with the the current 0.19.123 release so it seems like this was a regression within the 0.20.x milestone. We ran through the three different platforms and it looks like it's working fine so I'm okay with adding this into this milestone. I'm assuming we got this fix for free when we bumped muon?

Thoughts?

@bsclifton bsclifton modified the milestones: 0.19.x Hotfix 11 (Release Channel), 0.20.x (Beta Channel) Jan 4, 2018
@bsclifton
Copy link
Member

Moving back to 0.20.x - this may have never been a problem in 0.19.x... but there was a related Muon change (which is why I pulled to 0.19.x)

@kjozwiak
Copy link
Member

kjozwiak commented Jan 4, 2018

Clearing the flags as these were tested against 0.19.124. As mentioned above, this wasn't a regression within 0.19.x so this issue was moved back into the 0.20.x milestone.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.