Skip to content

Conversation

@slak44
Copy link

@slak44 slak44 commented Dec 6, 2022

  • What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)

Bug fix.

  • What is the current behavior? (You can also link to an open issue here)

Currently, there are several situations where undefined point objects are used in touch event handlers:

  1. Without touching the viewport beforehand, use the zoom tool. The panend event handler is called with startPoints undefined, which later throws.
  2. Using the magnify tool too quickly after scrolling the stack tries to draw the tool but evt.detail.currentPoints is undefined.
  3. There's also a way to reach the panmove handler with the lastDelta object undefined. Though I can't reproduce this as easily, the fix is similar.
  • What is the new behavior (if this is a feature change)?

Check that all these objects are defined, so the tools don't end up accessing properties on undefined. There was already one such check in panend, but the same problem shows up in a few places.

  • Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)

No.

  • Other information:

Found the issues by using the tools in OHIF.

@codecov
Copy link

codecov bot commented Dec 6, 2022

Codecov Report

Merging #1517 (e11001a) into master (925a3dd) will decrease coverage by 0.00%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master    #1517      +/-   ##
==========================================
- Coverage   21.70%   21.69%   -0.01%     
==========================================
  Files         287      287              
  Lines       10137    10139       +2     
  Branches     2081     2082       +1     
==========================================
  Hits         2200     2200              
- Misses       6750     6751       +1     
- Partials     1187     1188       +1     
Impacted Files Coverage Δ
src/eventListeners/touchEventListeners.js 4.27% <0.00%> (-0.05%) ⬇️
src/tools/MagnifyTool.js 0.00% <0.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant