Skip to content

Commit

Permalink
fix: setViewport can cause a crash if voi or scale are not provided (#…
Browse files Browse the repository at this point in the history
…493)

* be safer when merging options in setViewport

* added a test

* Update comment in test

* remove yarn.lock so travis can run
  • Loading branch information
mikehazell authored Sep 13, 2021
1 parent 0629c93 commit 4468b1b
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 2 deletions.
4 changes: 2 additions & 2 deletions src/setViewport.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,12 @@ export default function (element, viewport) {
// Prevent window width from being too small (note that values close to zero are valid and can occur with
// PET images in particular)
if (enabledElement.viewport.voi.windowWidth) {
enabledElement.viewport.voi.windowWidth = Math.max(viewport.voi.windowWidth, MIN_WINDOW_WIDTH);
enabledElement.viewport.voi.windowWidth = Math.max(enabledElement.viewport.voi.windowWidth, MIN_WINDOW_WIDTH);
}

// Prevent scale from getting too small
if (enabledElement.viewport.scale) {
enabledElement.viewport.scale = Math.max(viewport.scale, MIN_VIEWPORT_SCALE);
enabledElement.viewport.scale = Math.max(enabledElement.viewport.scale, MIN_VIEWPORT_SCALE);
}

// Normalize the rotation value to a positive rotation in degrees
Expand Down
22 changes: 22 additions & 0 deletions test/setViewport_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,28 @@ describe('Set an enabled element\'s viewport', function () {
});
});

it('all properties should be optional', function () {

// Act
displayImage(this.element, this.image, this.viewport);

const element = this.element;
const initialViewport = getViewport(element);

const newViewport = {};

// Act
setViewport(element, newViewport);

// Assert
const viewport = getViewport(element);

// We expect all the properties to be the same but we're really testing that the method did
// not throw by unintentionally relying on a property existing on the input.
assert.deepEqual(initialViewport, viewport);

});

it('should prevent windowWidth from getting too small', function () {

// Act
Expand Down

0 comments on commit 4468b1b

Please sign in to comment.