Skip to content

Commit

Permalink
Fix: Load images via img elements to avoid CORS issues with XHRs (#988)
Browse files Browse the repository at this point in the history
  • Loading branch information
@jstoffan
jstoffan authored Apr 18, 2019
1 parent 38b8fad commit fa6e82a
Show file tree
Hide file tree
Showing 6 changed files with 34 additions and 77 deletions.
8 changes: 3 additions & 5 deletions src/lib/viewers/image/ImageBaseViewer.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,6 @@ const CSS_CLASS_ZOOMABLE = 'zoomable';
const CSS_CLASS_PANNABLE = 'pannable';

class ImageBaseViewer extends BaseViewer {
/** @property {string} - Url used to download an image representation */
downloadUrl;

/** @inheritdoc */
constructor(options) {
super(options);
Expand Down Expand Up @@ -323,8 +320,9 @@ class ImageBaseViewer extends BaseViewer {
// eslint-disable-next-line
console.error(err);

const genericDownloadError = new PreviewError(ERROR_CODE.CONTENT_DOWNLOAD, __('error_refresh'), err.message);
super.handleDownloadError(err instanceof PreviewError ? err : genericDownloadError, imgUrl);
// Display a generic error message but log the real one
const error = new PreviewError(ERROR_CODE.CONTENT_DOWNLOAD, __('error_refresh'), {}, err.message);
super.handleDownloadError(error, imgUrl);
}

/**
Expand Down
18 changes: 6 additions & 12 deletions src/lib/viewers/image/ImageViewer.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,12 +57,12 @@ class ImageViewer extends ImageBaseViewer {

const { representation, viewer } = this.options;
const template = representation.content.url_template;
this.downloadUrl = this.createContentUrlWithAuthParams(template, viewer.ASSET);
const downloadUrl = this.createContentUrlWithAuthParams(template, viewer.ASSET);

this.bindDOMListeners();
return this.getRepStatus()
.getPromise()
.then(this.handleAssetAndRepLoad)
.then(() => this.handleAssetAndRepLoad(downloadUrl))
.catch(this.handleAssetError);
}

Expand All @@ -72,17 +72,11 @@ class ImageViewer extends ImageBaseViewer {
* @override
* @return {void}
*/
handleAssetAndRepLoad() {
handleAssetAndRepLoad(downloadUrl) {
this.startLoadTimer();
this.imageEl.src = downloadUrl;

return util
.fetchRepresentationAsBlob(this.downloadUrl)
.then((blob) => {
const srcUrl = URL.createObjectURL(blob);
this.imageEl.src = srcUrl;
super.handleAssetAndRepLoad();
})
.catch(this.handleImageDownloadError);
super.handleAssetAndRepLoad();
}

/**
Expand Down Expand Up @@ -380,7 +374,7 @@ class ImageViewer extends ImageBaseViewer {
* @return {void}
*/
handleImageDownloadError(err) {
this.handleDownloadError(err, this.downloadUrl);
this.handleDownloadError(err, this.imageEl.src);
}

/**
Expand Down
21 changes: 6 additions & 15 deletions src/lib/viewers/image/MultiImageViewer.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import PageControls from '../../PageControls';
import './MultiImage.scss';
import { ICON_FULLSCREEN_IN, ICON_FULLSCREEN_OUT } from '../../icons/icons';
import { CLASS_INVISIBLE, CLASS_MULTI_IMAGE_PAGE, CLASS_IS_SCROLLABLE } from '../../constants';
import { pageNumberFromScroll, fetchRepresentationAsBlob } from '../../util';
import { pageNumberFromScroll } from '../../util';

const PADDING_BUFFER = 100;
const CSS_CLASS_IMAGE = 'bp-images';
Expand Down Expand Up @@ -148,15 +148,7 @@ class MultiImageViewer extends ImageBaseViewer {
// Set page number. Page is index + 1.
this.singleImageEls[index].setAttribute('data-page-number', index + 1);
this.singleImageEls[index].classList.add(CLASS_MULTI_IMAGE_PAGE);

this.downloadUrl = imageUrl;

fetchRepresentationAsBlob(imageUrl)
.then((blob) => {
const srcUrl = URL.createObjectURL(blob);
this.singleImageEls[index].src = srcUrl;
})
.catch(this.handleMultiImageDownloadError);
this.singleImageEls[index].src = imageUrl;
}

/** @inheritdoc */
Expand Down Expand Up @@ -275,18 +267,17 @@ class MultiImageViewer extends ImageBaseViewer {
* @return {void}
*/
handleMultiImageDownloadError(err) {
if (this.isDestroyed()) {
return;
}

this.singleImageEls.forEach((el, index) => {
this.unbindImageListeners(index);
});

// Since we're using the src to get the hostname, we can always use the src of the first page
const { src } = this.singleImageEls[0];

// Clear any images we may have started to load.
this.singleImageEls = [];

this.handleDownloadError(err, this.downloadUrl);
this.handleDownloadError(err, src);
}

/**
Expand Down
27 changes: 13 additions & 14 deletions src/lib/viewers/image/__tests__/ImageViewer-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -236,8 +236,6 @@ describe('lib/viewers/image/ImageViewer', () => {
image.wrapperEl.style.height = '50px';

sandbox.stub(image, 'getRepStatus').returns({ getPromise: () => Promise.resolve() });
sandbox.stub(util, 'fetchRepresentationAsBlob').returns(Promise.resolve(imageUrl));
sandbox.stub(URL, 'createObjectURL').returns(imageUrl);
image.setup();
image.load(imageUrl).catch(() => {});
});
Expand Down Expand Up @@ -275,18 +273,21 @@ describe('lib/viewers/image/ImageViewer', () => {
});
});

it('should increase scale when the image is rotated', () => {
it('should zoom the width & height when the image rotated', () => {
sandbox.stub(image, 'isRotated').returns(true);
sandbox.stub(image, 'getTransformWidthAndHeight').returns({
width: 100,
height: 100
});
stubs.setScale = sandbox.stub(image, 'setScale');

image.imageEl.style.transform = 'rotate(90deg)';
image.imageEl.style.width = '200px';
image.imageEl.setAttribute('originalWidth', '150');
image.imageEl.setAttribute('originalHeight', '100');
image.imageEl.src = imageUrl;
const origImageSize = image.imageEl.getBoundingClientRect();
image.zoomIn();

expect(stubs.setScale).to.be.calledWith(undefined, 120);
const newImageSize = image.imageEl.getBoundingClientRect();
expect(newImageSize.width).gt(origImageSize.width);
expect(newImageSize.height).gt(origImageSize.height);
expect(stubs.adjustZoom).to.be.called;
image.imageEl.style.transform = '';
});

it('should reset dimensions and adjust padding when called with reset', () => {
Expand Down Expand Up @@ -598,13 +599,11 @@ describe('lib/viewers/image/ImageViewer', () => {
done();
})
);
sandbox.stub(URL, 'URL.createObjectURL(blob)').returns(url);

image.handleAssetAndRepLoad(url).then(() => {
expect(imageEl.src).to.be.equal(url);
});
image.handleAssetAndRepLoad(url);

expect(startLoadTimer).to.be.called;
expect(imageEl.url).to.be.equal(url);
expect(loadBoxAnnotations).to.be.called;
expect(createAnnotator).to.be.called;
});
Expand Down
25 changes: 5 additions & 20 deletions src/lib/viewers/image/__tests__/MultiImageViewer-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -210,17 +210,11 @@ describe('lib/viewers/image/MultiImageViewer', () => {
expect(stubs.bindImageListeners).to.be.called;
});

it('should set the download URL and fetch the page as a blob', () => {
stubs.fetchRepresentationAsBlob = sandbox
.stub(util, 'fetchRepresentationAsBlob')
.returns(Promise.resolve('foo'));
it('should set the image source', () => {
multiImage.singleImageEls = [stubs.singleImageEl];
const repUrl = 'file/100/content/{page}.png';

multiImage.setupImageEls(repUrl, 0);

expect(multiImage.downloadUrl).to.be.equal(repUrl);
expect(stubs.fetchRepresentationAsBlob).to.be.called;
multiImage.setupImageEls('file/100/content/{page}.png', 0);
expect(multiImage.singleImageEls[0].src).to.be.equal('file/100/content/{page}.png');
});

it('should set the page number for each image el', () => {
Expand Down Expand Up @@ -410,22 +404,13 @@ describe('lib/viewers/image/MultiImageViewer', () => {
sandbox.stub(multiImage, 'unbindImageListeners');
});

it('should do nothing if the viewer is already destroyed', () => {
sandbox.stub(multiImage, 'isDestroyed').returns(true);
multiImage.downloadUrl = multiImage.singleImageEls[0];
multiImage.handleMultiImageDownloadError('err');

expect(multiImage.handleDownloadError).to.not.be.called;
expect(multiImage.unbindImageListeners).to.not.be.called;
});

it('unbind the image listeners, clear the image Els array, and handle the download error', () => {
multiImage.downloadUrl = multiImage.singleImageEls[0];
const { src } = multiImage.singleImageEls[0];

multiImage.handleMultiImageDownloadError('err');

expect(multiImage.singleImageEls).to.deep.equal([]);
expect(multiImage.handleDownloadError).to.be.calledWith('err', multiImage.downloadUrl);
expect(multiImage.handleDownloadError).to.be.calledWith('err', src);
expect(multiImage.unbindImageListeners).to.be.calledTwice;
});
});
Expand Down
12 changes: 1 addition & 11 deletions test/integration/sanity/DeletedReps.e2e.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,7 @@ describe('Previewing a file with deleted representations', () => {
const token = Cypress.env('ACCESS_TOKEN');

const fileIdDoc = Cypress.env('FILE_ID_DOC');
const fileIdImage = Cypress.env('FILE_ID_IMAGE');
const fileIdPresentation = Cypress.env('FILE_ID_PRESENTATION');
const fileIdTiff = Cypress.env('FILE_ID_TIFF');

const REPS_ERROR = 'error_deleted_reps';

Expand Down Expand Up @@ -55,16 +53,8 @@ describe('Previewing a file with deleted representations', () => {
{
viewer: 'Presentation',
fileId: fileIdPresentation
},
{
viewer: 'Image',
fileId: fileIdImage
},
{
viewer: 'MultiImage',
fileId: fileIdTiff
}
].forEach((test) => {
].forEach((test) => {
it(test.viewer, () => {
helpers.checkDeletedRepError();

Expand Down

0 comments on commit fa6e82a

Please sign in to comment.