Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix out of bounds error #9 #10

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

weihao
Copy link

@weihao weihao commented Apr 14, 2021

Problem

In the original short-circuit expression

this.pageX = event.pageX || event.targetTouches[0].pageX || event.originalEvent.targetTouches[0].pageX;

event.pageX could be number 0 which is a valid x location, but it was evaluated as false in the short-circuit and it would try to evaluate event.targetTouches[0].pageX which is undefined and caused an error. #9

Fix

this.pageX = +(x !== 0) && (x || event.targetTouches[0].pageX || event.originalEvent.targetTouches[0].pageX)

Case 1, x is 0

+(x !== 0) = +(false) = 0 && (x || event.targetTouches[0].pageX || event.originalEvent.targetTouches[0].pageX) = 0
this.pageX = 0

Case 2, x is non-zero value, like 2

+(x !== 0) = +(true) = 1 && (x || event.targetTouches[0].pageX || event.originalEvent.targetTouches[0].pageX) = x
this.pageX = 2

Case 3, x is null

+(x !== 0) = +(true) = 1 && (x || event.targetTouches[0].pageX || event.originalEvent.targetTouches[0].pageX) = event.targetTouches[0].pageX || event.originalEvent.targetTouches[0].pageX
this.pageX = event.targetTouches[0].pageX || event.originalEvent.targetTouches[0].pageX

@weihao weihao mentioned this pull request Apr 14, 2021
@marcincichocki
Copy link
Owner

Thanks for contributing! PR looks good, I'll merge it soon.

Unfortunately I'm busy at the moment so finding lost in time and space npm credentials, and actually deploying it to npm might take a little while. While I'm at it I would like to get this repo back to date with some tests and CI, so for now either use your own fork, or patch package locally with https://github.com/ds300/patch-package

You can track progress being made in #11

@weihao
Copy link
Author

weihao commented Apr 14, 2021

thanks for the pointer for patching node modules. that's very helpful!

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.

2 participants