Fix: Hiding download button in all cases if browser cannot download (#61

)

src/lib/Browser.js

@@ -235,7 +235,7 @@ class Browser {
      * @return {boolean} true if browser supports download
      */
     static canDownload() {
-        return !window.externalHost && 'download' in document.createElement('a');
+        return !Browser.isMobile() || (!window.externalHost && 'download' in document.createElement('a'));
     }
 
     /**

src/lib/Preview.js

@@ -830,8 +830,8 @@ class Preview extends EventEmitter {
             throw new Error(__('error_permissions'));
         }
 
-        // Show download button if download permissions exist and preview options allow
-        if (checkPermission(this.file, PERMISSION_DOWNLOAD) && this.options.showDownload) {
+        // Show download button if download permissions exist, options allow, and browser has ability
+        if (checkPermission(this.file, PERMISSION_DOWNLOAD) && this.options.showDownload && Browser.canDownload()) {
             showLoadingDownloadButton(this.download);
         }
 
@@ -916,7 +916,7 @@ class Preview extends EventEmitter {
     finishLoading(data = {}) {
         // Show or hide annotate/print/download buttons
         // canDownload is not supported by all of our browsers, so for now we need to check isMobile
-        if (checkPermission(this.file, PERMISSION_DOWNLOAD) && this.options.showDownload && (Browser.canDownload() || !Browser.isMobile())) {
+        if (checkPermission(this.file, PERMISSION_DOWNLOAD) && this.options.showDownload && Browser.canDownload()) {
             showDownloadButton(this.download);
 
             if (checkFeature(this.viewer, 'print') && !Browser.isMobile()) {

src/lib/__tests__/Preview-test.js

@@ -1122,6 +1122,7 @@ describe('lib/Preview', () => {
 
             stubs.destroy = sandbox.stub(preview, 'destroy');
             stubs.checkPermission = sandbox.stub(file, 'checkPermission').returns(true);
+            stubs.canDownload = sandbox.stub(Browser, 'canDownload').returns(false);
             stubs.showLoadingDownloadButton = sandbox.stub(ui, 'showLoadingDownloadButton');
             stubs.loadPromiseResolve = Promise.resolve();
             stubs.determineRepresentationStatusPromise = Promise.resolve();
@@ -1167,7 +1168,7 @@ describe('lib/Preview', () => {
             }
         });
 
-        it('should show the loading download button if there are sufficient permissions', () => {
+        it('should show the loading download button if there are sufficient permissions and support', () => {
             stubs.checkPermission.withArgs(sinon.match.any, 'can_download').returns(false);
             preview.options.showDownload = false;
 
@@ -1190,6 +1191,16 @@ describe('lib/Preview', () => {
 
             stubs.checkPermission.withArgs(sinon.match.any, 'can_download').returns(true);
             preview.options.showDownload = true;
+            stubs.canDownload.returns(false);
+            preview.destroy();
+
+            preview.loadViewer({});
+            expect(stubs.showLoadingDownloadButton).to.not.be.called;
+
+            stubs.checkPermission.withArgs(sinon.match.any, 'can_download').returns(true);
+            preview.options.showDownload = true;
+            stubs.canDownload.returns(true);
+
 
             preview.loadViewer({});
             expect(stubs.showLoadingDownloadButton).to.be.called;
@@ -1300,7 +1311,7 @@ describe('lib/Preview', () => {
             stubs.checkFeature.returns(true);
         });
 
-        it('should show download button if there is download permission', () => {
+        it('should only show download button if there is download permission', () => {
             stubs.checkPermission.returns(false);
 
             preview.finishLoading();
@@ -1328,13 +1339,11 @@ describe('lib/Preview', () => {
 
         it('should show download button if download is supported by browser', () => {
             stubs.canDownload.returns(false);
-            stubs.isMobile.returns(true);
 
             preview.finishLoading();
             expect(stubs.showDownloadButton).to.not.be.called;
 
-            stubs.canDownload.returns(false);
-            stubs.isMobile.returns(false);
+            stubs.canDownload.returns(true);
 
             preview.finishLoading();
             expect(stubs.showDownloadButton).to.be.called;

src/lib/viewers/error/PreviewErrorViewer.js

@@ -1,6 +1,7 @@
 import autobind from 'autobind-decorator';
 import BaseViewer from '../BaseViewer';
 import { checkPermission } from '../../file';
+import Browser from '../../Browser';
 import { PERMISSION_DOWNLOAD } from '../../constants';
 import { ICON_FILE_DEFAULT, ICON_FILE_MEDIA, ICON_FILE_ZIP } from '../../icons/icons';
 import './PreviewError.scss';
@@ -70,7 +71,7 @@ class PreviewErrorViewer extends BaseViewer {
         this.messageEl.textContent = message;
 
         // Add optional download button
-        if (checkPermission(file, PERMISSION_DOWNLOAD) && showDownload) {
+        if (checkPermission(file, PERMISSION_DOWNLOAD) && showDownload && Browser.canDownload()) {
             this.addDownloadButton();
         }
 

src/lib/viewers/error/__tests__/PreviewErrorViewer-test.js

@@ -1,5 +1,6 @@
 /* eslint-disable no-unused-expressions */
 import PreviewErrorViewer from '../PreviewErrorViewer';
+import Browser from '../../../Browser';
 import * as file from '../../../file';
 import { PERMISSION_DOWNLOAD } from '../../../constants';
 import {
@@ -97,6 +98,15 @@ describe('lib/viewers/error/PreviewErrorViewer', () => {
             expect(error.addDownloadButton).to.not.be.called;
         });
 
+        it('should not add download button if the browser cannot download', () => {
+            sandbox.stub(error, 'addDownloadButton');
+            sandbox.stub(Browser, 'canDownload').returns(false);
+
+            error.load('reason');
+
+            expect(error.addDownloadButton).to.not.be.called;
+        });
+
         it('should broadcast load', () => {
             sandbox.stub(error, 'emit');