fix(BaseBrushTool.js): prevents a crash in passiveCallback #925
Conversation
prevents a crash when _repeatGlobalToolHistory replays setting tool mode passive for brush tools fix cornerstonejs#924
Codecov Report
@@ Coverage Diff @@
## master #925 +/- ##
==========================================
- Coverage 11.73% 11.73% -0.01%
==========================================
Files 218 218
Lines 7157 7159 +2
Branches 1124 1124
==========================================
Hits 840 840
- Misses 5342 5344 +2
Partials 975 975
Continue to review full report at Codecov.
|
| @@ -141,7 +141,13 @@ class BaseBrushTool extends BaseTool { | |||
| */ | |||
| // eslint-disable-next-line no-unused-vars | |||
| passiveCallback(evt) { | |||
| external.cornerstone.updateImage(this.element); | |||
| try { | |||
| external.cornerstone.updateImage(this.element); | |||
There was a problem hiding this comment.
I really wish updateImage and similar methods were safe to call with elements that don't have a loaded image or are not called with an enabled element.
There was a problem hiding this comment.
Totally agree.
Particularly updateImage. It doesn't access the image itself, it just calls drawImage which also doesn't touch the image but sets enabledElement.needsRedraw.
needsRedraw is then checked in the run-loop which also checks to make sure the image is loaded.
There's nothing here that would break if updateImage was called when no image is loaded.
|
LGTM 👍 |
|
🎉 This PR is included in version 3.7.3 🎉 The release is available on: Your semantic-release bot 📦🚀 |
prevents a crash when _repeatGlobalToolHistory replays setting tool mode passive for brush tools
fix #924
Bug fix
Crashing Bug: using the Brush tool with
globalToolSyncEnabledcan cause the application to crash when a new element is enabled._repeatGlobalToolHistoryreplays all tool changes on a new element before the image is loadedcornerstone.updateImagein it'spassiveCallbackcornerstone.updateImagewill crash if the image is not loadedWrapped the call to
updateImagein atry ... catch.This should be seen as a temporary fix. A more permanent fix could be to keep track of the current tool state instead of recording and replaying changes. Alternatively, there appears to be no reason why
cornerstone.updateImageneed to crash when the image is not loaded so changing this behaviour could be more effective.No