jspaint
jspaint copied to clipboard
Error when changing eraser size with keyboard
Note: I already have a fix for this. (I just wanted a space to gather my thoughts without framing it in the past tense, mainly.)
When pressing numpad plus/minus (or regular +/-) to increase or decrease the size of the eraser, it can throw an error:
TypeError: pointer_previous is undefined
paint@https://jspaint.app/src/tools.js:366:19
tool_go@https://jspaint.app/src/app.js:1460:17
@https://jspaint.app/src/app.js:1044:13
@https://jspaint.app/src/app.js:1043:20
dispatch@https://jspaint.app/lib/jquery-3.4.1.min.js:2:42571
or:
TypeError: this.mask_canvas is null
paint_iteration@https://jspaint.app/src/tools.js:377:5
paint/<@https://jspaint.app/src/tools.js:367:10
bresenham_line@https://jspaint.app/src/image-manipulation.js:232:11
paint@https://jspaint.app/src/tools.js:366:18
tool_go@https://jspaint.app/src/app.js:1460:17
@https://jspaint.app/src/app.js:1044:13
@https://jspaint.app/src/app.js:1043:20
dispatch@https://jspaint.app/lib/jquery-3.4.1.min.js:2:42571
To reproduce the errors:
- Select a color in the palette (so that
buttonis set) - Hover over the canvas (so that
pointeris set) - Optionally draw on the canvas. This changes which error is thrown (because
pointer_previousgets set) - Select the Eraser tool
- Press numpad +/-
Looking through the usages of button and ctrl in the codebase, I don't see any reason why these globals should be set when clicking on a color well. Perhaps it was out of an ill-conceived notion of logical consistency, or blind copy pasting, or, heavens forbid, the slight brevity of dropping e. used to access the state from the event object. Perhaps the Edit Colors dialog used these before refactoring it to determine the color_selection_slot externally?
Digging up the code with git blame, this code's been in the project for ten years. It was introduced here.
This was before the Edit Colors dialog, when it used the browser's native color picker. It looks like it was using it to persist the state while selecting a color with the native color picker. Since it's before I implemented the Edit Colors dialog, there's no function call to add a parameter to which would hold the state. It uses the change event of an <input type="color">, so it needed a variable to store which mouse button was used, and thus which color selection to change, and I may have used the existing globals button and ctrl out of convenience. So it's similar to my last theory, but on a longer time frame.
And yes, I have a global called button, it's pretty bad.