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

feat(office): Preview PDF rep if using internet explorer #1416

Merged
merged 5 commits into from
Aug 10, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/i18n/en-US.properties
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,8 @@ error_bad_file=We’re sorry the preview didn’t load. This file could not be c
error_not_downloadable=Oops! It looks like something is wrong with this file. We apologize for the inconvenience and recommend that you upload a new version of this file or roll back to a previous version. Please contact us for more details.
# Preview error message shown when flash is not enabled on their browser
error_flash_not_enabled=We’re sorry, the preview didn’t load because Flash is not enabled on your browser. If possible, please enable Flash and refresh the page.
# Preview error message shown when loading what would be an Office Excel Online file extension in Internet Explorer
error_internet_explorer_office_online=Microsoft 365 apps and services no longer support Internet Explorer 11, therefore, Box users may have a degraded experience.

# Archive Preview
# Label for filename column name
Expand Down
8 changes: 8 additions & 0 deletions src/lib/Browser.js
Original file line number Diff line number Diff line change
Expand Up @@ -352,6 +352,14 @@ class Browser {
},
};
}

/**
* Returns whether the browser is Internet Explorer
* @returns {boolean} - Whether the browser is Internet Explorer
*/
static isIE() {
return Browser.getName() === BROWSERS.INTERNET_EXPLORER;
}
}

export default Browser;
16 changes: 16 additions & 0 deletions src/lib/viewers/doc/DocumentViewer.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import Browser from '../../Browser';
import DocBaseViewer from './DocBaseViewer';
import DocPreloader from './DocPreloader';
import fullscreen from '../../Fullscreen';
import { EXCEL_EXTENSIONS } from '../../extensions';
import './Document.scss';

class DocumentViewer extends DocBaseViewer {
Expand Down Expand Up @@ -33,6 +35,20 @@ class DocumentViewer extends DocBaseViewer {
this.preloader.removeAllListeners('preload');
}

/**
* @inheritdoc
*/
load() {
super.load();

const { extension } = this.options.file;
const isExcelExtension = EXCEL_EXTENSIONS.includes(extension);

if (isExcelExtension && Browser.isIE()) {
this.options.ui.showNotification(__('error_internet_explorer_office_online'), null, true);
}
}

/**
* Handles keyboard events for document viewer.
*
Expand Down
74 changes: 74 additions & 0 deletions src/lib/viewers/doc/__tests__/DocumentViewer-test.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
/* eslint-disable no-unused-expressions */
import Api from '../../../api';
import Browser from '../../../Browser';
import DocumentViewer from '../DocumentViewer';
import DocBaseViewer from '../DocBaseViewer';
import BaseViewer from '../../BaseViewer';
Expand All @@ -15,6 +17,8 @@ describe('lib/viewers/doc/DocumentViewer', () => {
beforeEach(() => {
fixture.load('viewers/doc/__tests__/DocumentViewer-test.html');

stubs.api = new Api();

containerEl = document.querySelector('.container');
doc = new DocumentViewer({
container: containerEl,
Expand Down Expand Up @@ -83,6 +87,76 @@ describe('lib/viewers/doc/DocumentViewer', () => {
});
});

describe('load()', () => {
const docBaseLoadFunc = DocBaseViewer.prototype.load;

beforeEach(() => {
jest.spyOn(stubs.api, 'get').mockImplementation();
jest.spyOn(doc, 'setup').mockImplementation();
Object.defineProperty(DocBaseViewer.prototype, 'load', {
value: jest.fn().mockImplementation(() => Promise.resolve()),
});
jest.spyOn(doc, 'createContentUrlWithAuthParams').mockImplementation();
jest.spyOn(doc, 'handleAssetAndRepLoad').mockImplementation();
jest.spyOn(doc, 'getRepStatus').mockReturnValue({ getPromise: () => Promise.resolve() });
jest.spyOn(doc, 'loadAssets').mockImplementation();
jest.spyOn(doc, 'loadBoxAnnotations').mockImplementation();
});

afterEach(() => {
Object.defineProperty(DocBaseViewer.prototype, 'load', { value: docBaseLoadFunc });
});

test.each(['xls', 'xlsm', 'xlsx'])(
'should show notification if file has excel extension %s and is internet explorer',
extension => {
const showNotification = jest.fn();
doc.options.ui = { showNotification };
doc.options.file.extension = extension;
jest.spyOn(Browser, 'isIE').mockImplementation(() => true);

doc.load();

expect(showNotification).toBeCalledWith(__('error_internet_explorer_office_online'), null, true);
},
);

test.each(['xls', 'xlsm', 'xlsx'])(
'should not show notification if file has excel extension %s but is not internet explorer',
extension => {
const showNotification = jest.fn();
doc.options.ui = { showNotification };
doc.options.file.extension = extension;
jest.spyOn(Browser, 'isIE').mockImplementation(() => false);

doc.load();

expect(showNotification).not.toBeCalledWith(__('error_internet_explorer_office_online'), null, true);
},
);

test('should not show notification if file is not excel extension', () => {
const showNotification = jest.fn();
doc.options.ui = { showNotification };
doc.options.file.extension = 'pdf';

doc.load();

expect(showNotification).not.toBeCalledWith(__('error_internet_explorer_office_online'), null, true);
});

test('should not show notification if file is not excel extension and is internet explorer', () => {
const showNotification = jest.fn();
doc.options.ui = { showNotification };
doc.options.file.extension = 'pdf';
jest.spyOn(Browser, 'isIE').mockImplementation(() => true);

doc.load();

expect(showNotification).not.toBeCalledWith(__('error_internet_explorer_office_online'), null, true);
});
});

describe('onKeydown()', () => {
beforeEach(() => {
stubs.zoomIn = jest.spyOn(doc, 'zoomIn').mockImplementation();
Expand Down
4 changes: 3 additions & 1 deletion src/lib/viewers/office/OfficeLoader.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import getProp from 'lodash/get';
import AssetLoader from '../AssetLoader';
import Browser from '../../Browser';
import OfficeViewer from './OfficeViewer';
import { checkPermission } from '../../file';
import { ORIGINAL_REP_NAME, PERMISSION_DOWNLOAD } from '../../constants';
Expand Down Expand Up @@ -39,7 +40,8 @@ class OfficeLoader extends AssetLoader {
if (
!checkPermission(file, PERMISSION_DOWNLOAD) ||
file.size > maxFileSize ||
isDisabledDueToPasswordProtectedSharedLink
isDisabledDueToPasswordProtectedSharedLink ||
Browser.isIE() // Disable Office viewer if browser is Internet Explorer due to EOL
) {
disabledViewers.push(OFFICE_VIEWER_NAME);
}
Expand Down
9 changes: 8 additions & 1 deletion src/lib/viewers/office/__tests__/OfficeLoader-test.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import * as file from '../../../file';
import Browser from '../../../Browser';
import OfficeLoader from '../OfficeLoader';
import OfficeViewer from '../OfficeViewer';
import * as file from '../../../file';

const FIVE_MB = 5242880;

Expand Down Expand Up @@ -86,6 +87,12 @@ describe('lib/viewers/office/OfficeLoader', () => {
const viewer = OfficeLoader.determineViewer(editedFakeFile, [], viewerOptions);
expect(viewer).toBeDefined();
});

test('should not return a viewer if the browser is internet explorer', () => {
jest.spyOn(Browser, 'isIE').mockImplementation(() => true);
const viewer = OfficeLoader.determineViewer(fakeFile, []);
expect(viewer).toBeUndefined();
});
});
});
});