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

Commit 498bc75

Browse files
committed
move pdf.js loading logic into browser-laptop
fix #4651 probably fixes #2715 but ideally the workaround in brave/pdf.js should be removed too may also fix other PDF loading errors
1 parent 0147755 commit 498bc75

File tree

4 files changed

+126
-17
lines changed

4 files changed

+126
-17
lines changed

app/extensions.js

+20-7
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ const contextMenus = require('./browser/extensions/contextMenus')
33
const extensionActions = require('./common/actions/extensionActions')
44
const config = require('../js/constants/config')
55
const appConfig = require('../js/constants/appConfig')
6-
const messages = require('../js/constants/messages')
76
const {fileUrl} = require('../js/lib/appUrlUtil')
87
const {getExtensionsPath, getBraveExtUrl, getBraveExtIndexHTML} = require('../js/lib/appUrlUtil')
98
const {getSetting} = require('../js/settings')
@@ -17,7 +16,7 @@ const fs = require('fs')
1716
const path = require('path')
1817
const l10n = require('../js/l10n')
1918
const {bravifyText} = require('./renderer/lib/extensionsUtil')
20-
const {ipcMain, componentUpdater, session} = require('electron')
19+
const {componentUpdater, session} = require('electron')
2120

2221
// Takes Content Security Policy flags, for example { 'default-src': '*' }
2322
// Returns a CSP string, for example 'default-src: *;'
@@ -392,10 +391,6 @@ module.exports.init = () => {
392391
}
393392
})
394393

395-
ipcMain.on(messages.LOAD_URL_REQUESTED, (e, tabId, url) => {
396-
appActions.loadURLRequested(tabId, url)
397-
})
398-
399394
process.on('extension-load-error', (error) => {
400395
console.error(error)
401396
})
@@ -447,8 +442,26 @@ module.exports.init = () => {
447442
}
448443

449444
let loadExtension = (extensionId, extensionPath, manifest = {}, manifestLocation = 'unpacked') => {
450-
if (extensionId === config.PDFJSExtensionId) {
445+
const isPDF = extensionId === config.PDFJSExtensionId
446+
if (isPDF) {
451447
manifestLocation = 'component'
448+
// Override the manifest. TODO: Update manifest in pdf.js itself after enough
449+
// clients have updated to Brave 0.20.x+
450+
manifest = {
451+
name: 'PDF Viewer',
452+
manifest_version: 2,
453+
version: '1.9.457',
454+
description: 'Uses HTML5 to display PDF files directly in the browser.',
455+
icons: {
456+
'128': 'icon128.png',
457+
'48': 'icon48.png',
458+
'16': 'icon16.png'
459+
},
460+
permissions: ['storage', '<all_urls>'],
461+
content_security_policy: "script-src 'self'; object-src 'self'",
462+
incognito: 'split',
463+
key: config.PDFJSExtensionPublicKey
464+
}
452465
}
453466
if (!extensionInfo.isLoaded(extensionId) && !extensionInfo.isLoading(extensionId)) {
454467
extensionInfo.setState(extensionId, extensionStates.LOADING)

app/pdfJS.js

+88-8
Original file line numberDiff line numberDiff line change
@@ -5,23 +5,102 @@
55

66
const UrlUtil = require('../js/lib/urlutil')
77
const Filtering = require('./filtering')
8-
const config = require('../js/constants/config')
98
const appActions = require('../js/actions/appActions')
109
const getSetting = require('../js/settings').getSetting
1110
const settings = require('../js/constants/settings')
1211

13-
const pdfjsBaseUrl = `chrome-extension://${config.PDFJSExtensionId}/`
14-
const viewerBaseUrl = `${pdfjsBaseUrl}content/web/viewer.html`
12+
const getViewerUrl = UrlUtil.getPDFViewerUrl
13+
14+
/**
15+
* Check if the request is a PDF file.
16+
* @param {Object} details First argument of the webRequest.onHeadersReceived
17+
* event. The properties "responseHeaders" and "url"
18+
* are read.
19+
* @return {boolean} True if the resource is a PDF file.
20+
*/
21+
function isPdfFile (details) {
22+
var header = details.responseHeaders['Content-Type']
23+
if (header) {
24+
if (header.includes('application/pdf')) {
25+
return true
26+
}
27+
if (header.includes('application/octet-stream')) {
28+
if (details.url.toLowerCase().indexOf('.pdf') > 0) {
29+
return true
30+
}
31+
var cdHeader = details.responseHeaders['Content-Disposition']
32+
if (cdHeader && /\.pdf(["']|$)/i.test(cdHeader[0])) {
33+
return true
34+
}
35+
}
36+
}
37+
}
38+
39+
/**
40+
* @param {Object} details First argument of the webRequest.onHeadersReceived
41+
* event. The property "url" is read.
42+
* @return {boolean} True if the PDF file should be downloaded.
43+
*/
44+
function isPdfDownloadable (details) {
45+
if (details.url.indexOf('pdfjs.action=download') >= 0) {
46+
return true
47+
}
48+
// Display the PDF viewer regardless of the Content-Disposition header if the
49+
// file is displayed in the main frame, since most often users want to view
50+
// a PDF, and servers are often misconfigured.
51+
// If the query string contains "=download", do not unconditionally force the
52+
// viewer to open the PDF, but first check whether the Content-Disposition
53+
// header specifies an attachment. This allows sites like Google Drive to
54+
// operate correctly (#6106).
55+
if (details.type === 'main_frame' &&
56+
details.url.indexOf('=download') === -1) {
57+
return false
58+
}
59+
var cdHeader = (details.responseHeaders &&
60+
details.responseHeaders['Content-Disposition'])
61+
return (cdHeader && /^attachment/i.test(cdHeader[0]))
62+
}
63+
64+
/**
65+
* Takes a set of headers, and set "Content-Disposition: attachment".
66+
* @param {Object} details First argument of the webRequest.onHeadersReceived
67+
* event. The property "responseHeaders" is read and
68+
* modified if needed.
69+
* @return {Object|undefined} The return value for the responseHeaders property
70+
*/
71+
function getHeadersWithContentDispositionAttachment (details) {
72+
var headers = details.responseHeaders
73+
var cdHeader = headers['Content-Disposition'] || []
74+
cdHeader.push('attachment')
75+
headers['Content-Disposition'] = cdHeader
76+
return headers
77+
}
1578

1679
const onBeforeRequest = (details) => {
1780
const result = { resourceName: 'pdfjs' }
18-
if (!(details.resourceType === 'mainFrame' &&
81+
if (details.resourceType === 'mainFrame' &&
1982
UrlUtil.isFileScheme(details.url) &&
20-
UrlUtil.isFileType(details.url, 'pdf'))) {
21-
return result
83+
UrlUtil.isFileType(details.url, 'pdf')) {
84+
appActions.loadURLRequested(details.tabId, getViewerUrl(details.url))
85+
result.cancel = true
86+
}
87+
return result
88+
}
89+
90+
const onHeadersReceived = (details) => {
91+
const result = { resourceName: 'pdfjs' }
92+
// Don't intercept POST requests until http://crbug.com/104058 is fixed.
93+
if (details.resourceType === 'mainFrame' && details.method === 'GET' && isPdfFile(details)) {
94+
if (isPdfDownloadable(details)) {
95+
// Force download by ensuring that Content-Disposition: attachment is set
96+
result.responseHeaders = getHeadersWithContentDispositionAttachment(details)
97+
return result
98+
}
99+
100+
// Replace frame with viewer
101+
appActions.loadURLRequested(details.tabId, getViewerUrl(details.url))
102+
result.cancel = true
22103
}
23-
appActions.loadURLRequested(details.tabId, `${viewerBaseUrl}?file=${encodeURIComponent(details.url)}`)
24-
result.cancel = true
25104
return result
26105
}
27106

@@ -31,5 +110,6 @@ const onBeforeRequest = (details) => {
31110
module.exports.init = () => {
32111
if (getSetting(settings.PDFJS_ENABLED)) {
33112
Filtering.registerBeforeRequestFilteringCB(onBeforeRequest)
113+
Filtering.registerHeadersReceivedFilteringCB(onHeadersReceived)
34114
}
35115
}

js/lib/urlutil.js

+7-1
Original file line numberDiff line numberDiff line change
@@ -322,6 +322,12 @@ const UrlUtil = {
322322
return url.replace(`chrome-extension://${pdfjsExtensionId}/`, '')
323323
},
324324

325+
getPDFViewerUrl: function (url) {
326+
const pdfjsBaseUrl = `chrome-extension://${pdfjsExtensionId}/`
327+
const viewerBaseUrl = `${pdfjsBaseUrl}content/web/viewer.html`
328+
return `${viewerBaseUrl}?file=${encodeURIComponent(url)}`
329+
},
330+
325331
/**
326332
* Converts a potential PDF URL to the PDFJS URL.
327333
* XXX: This only looks at the URL file extension, not MIME types.
@@ -330,7 +336,7 @@ const UrlUtil = {
330336
*/
331337
toPDFJSLocation: function (url) {
332338
if (url && UrlUtil.isHttpOrHttps(url) && UrlUtil.isFileType(url, 'pdf')) {
333-
return `chrome-extension://${pdfjsExtensionId}/${url}`
339+
return UrlUtil.getPDFViewerUrl(url)
334340
}
335341
return url
336342
},

test/unit/lib/urlutilTest.js

+11-1
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,7 @@ describe('urlutil', function () {
195195
describe('toPDFJSLocation', function () {
196196
const baseUrl = 'chrome-extension://jdbefljfgobbmcidnmpjamcbhnbphjnb/'
197197
it('pdf', function () {
198-
assert.equal(urlUtil.toPDFJSLocation('http://abc.com/test.pdf'), baseUrl + 'http://abc.com/test.pdf')
198+
assert.equal(urlUtil.toPDFJSLocation('http://abc.com/test.pdf'), baseUrl + 'content/web/viewer.html?file=http%3A%2F%2Fabc.com%2Ftest.pdf')
199199
})
200200
it('non-pdf', function () {
201201
assert.equal(urlUtil.toPDFJSLocation('http://abc.com/test.pdf.txt'), 'http://abc.com/test.pdf.txt')
@@ -208,6 +208,16 @@ describe('urlutil', function () {
208208
})
209209
})
210210

211+
describe('getPDFViewerUrl', function () {
212+
const baseUrl = 'chrome-extension://jdbefljfgobbmcidnmpjamcbhnbphjnb/content/web/viewer.html?file='
213+
it('regular url', function () {
214+
assert.equal(urlUtil.getPDFViewerUrl('http://example.com'), baseUrl + 'http%3A%2F%2Fexample.com')
215+
})
216+
it('file url', function () {
217+
assert.equal(urlUtil.getPDFViewerUrl('file:///Users/yan/some files/test.pdf'), baseUrl + 'file%3A%2F%2F%2FUsers%2Fyan%2Fsome%20files%2Ftest.pdf')
218+
})
219+
})
220+
211221
describe('getHostname', function () {
212222
it('returns undefined if the URL is invalid', function () {
213223
assert.equal(urlUtil.getHostname(null), undefined)

0 commit comments

Comments
 (0)