Fix brave#10554, brave#11059: remove check for .pdf in URL, prefer re…

…sponse headers.
humphd · Mar 23, 2018 · 173bc36 · 173bc36
1 parent 5bd5b15
commit 173bc36
Show file tree

Hide file tree

Showing 5 changed files with 11 additions and 33 deletions.
diff --git a/app/browser/tabs.js b/app/browser/tabs.js
@@ -11,7 +11,7 @@ const tabState = require('../common/state/tabState')
 const {app, BrowserWindow, extensions, session, ipcMain} = require('electron')
 const {makeImmutable, makeJS} = require('../common/state/immutableUtil')
 const {getTargetAboutUrl, getSourceAboutUrl, isSourceAboutUrl, newFrameUrl, isTargetAboutUrl, isIntermediateAboutPage, isTargetMagnetUrl, getSourceMagnetUrl} = require('../../js/lib/appUrlUtil')
-const {isURL, getUrlFromInput, toPDFJSLocation, getDefaultFaviconUrl, isHttpOrHttps, getLocationIfPDF} = require('../../js/lib/urlutil')
+const {isURL, getUrlFromInput, getDefaultFaviconUrl, isHttpOrHttps, getLocationIfPDF} = require('../../js/lib/urlutil')
 const {isSessionPartition} = require('../../js/state/frameStateUtil')
 const {getOrigin} = require('../../js/lib/urlutil')
 const settingsStore = require('../../js/settings')
@@ -50,9 +50,6 @@ const normalizeUrl = function (url) {
   if (isURL(url)) {
     url = getUrlFromInput(url)
   }
-  if (settingsStore.getSetting(settings.PDFJS_ENABLED)) {
-    url = toPDFJSLocation(url)
-  }
   return url
 }
 

diff --git a/js/lib/urlutil.js b/js/lib/urlutil.js
@@ -355,19 +355,6 @@ const UrlUtil = {
     return `${viewerBaseUrl}?file=${encodeURIComponent(url)}`
   },
 
-  /**
-   * Converts a potential PDF URL to the PDFJS URL.
-   * XXX: This only looks at the URL file extension, not MIME types.
-   * @param {string} url
-   * @return {string}
-   */
-  toPDFJSLocation: function (url) {
-    if (url && UrlUtil.isHttpOrHttps(url) && UrlUtil.isFileType(url, 'pdf')) {
-      return UrlUtil.getPDFViewerUrl(url)
-    }
-    return url
-  },
-
   /**
    * Gets the default favicon URL for a URL.
    * @param {string} url The URL to find a favicon for

diff --git a/test/fixtures/html_with_pdf_extension.pdf b/test/fixtures/html_with_pdf_extension.pdf
diff --git a/test/tab-components/frameTest.js b/test/tab-components/frameTest.js
@@ -2,6 +2,7 @@
 
 const Brave = require('../lib/brave')
 const {urlInput,
+  activeTabTitle,
   pinnedTabsTabs,
   backButtonEnabled,
   backButtonDisabled,
@@ -278,6 +279,15 @@ describe('frame tests', function () {
         .url(url)
         .waitForVisible('#viewerContainer')
     })
+
+    it('loads HTML properly despite having .pdf extension', function * () {
+      let url = Brave.server.url('html_with-pdf_extension.pdf')
+      yield this.app.client
+        .windowByUrl(Brave.browserWindowUrl)
+        .tabByIndex(0)
+        .loadUrl(url)
+        .waitForTextValue(activeTabTitle, 'HTML using .pdf Extension')
+    })
   })
 })
 

diff --git a/test/unit/lib/urlutilTest.js b/test/unit/lib/urlutilTest.js
@@ -192,22 +192,6 @@ describe('urlutil', function () {
     })
   })
 
-  describe('toPDFJSLocation', function () {
-    const baseUrl = 'chrome-extension://jdbefljfgobbmcidnmpjamcbhnbphjnb/'
-    it('pdf', function () {
-      assert.equal(urlUtil.toPDFJSLocation('http://abc.com/test.pdf'), baseUrl + 'content/web/viewer.html?file=http%3A%2F%2Fabc.com%2Ftest.pdf')
-    })
-    it('non-pdf', function () {
-      assert.equal(urlUtil.toPDFJSLocation('http://abc.com/test.pdf.txt'), 'http://abc.com/test.pdf.txt')
-    })
-    it('file url', function () {
-      assert.equal(urlUtil.toPDFJSLocation('file://abc.com/test.pdf.txt'), 'file://abc.com/test.pdf.txt')
-    })
-    it('empty', function () {
-      assert.equal(urlUtil.toPDFJSLocation(''), '')
-    })
-  })
-
   describe('getPDFViewerUrl', function () {
     const baseUrl = 'chrome-extension://jdbefljfgobbmcidnmpjamcbhnbphjnb/content/web/viewer.html?file='
     it('regular url', function () {