-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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(touch): prevent infinite loop on multi-touch drag #8470
Conversation
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Welcome! It looks like this is your first pull request in Blockly, so here are a couple of tips:
- You can find tips about contributing to Blockly and how to validate your changes on our developer site.
- All contributors must sign the Google Contributor License Agreement (CLA). If the google-cla bot leaves a comment on this PR, make sure you follow the instructions.
- We use conventional commits to make versioning the package easier. Make sure your commit message is in the proper format or learn how to fix it.
- If any of the other checks on this PR fail, you can click on them to learn why. It might be that your change caused a test failure, or that you need to double-check the style guide.
Thank you for opening this PR! A member of the Blockly team will review it soon.
Heya @AbhinavKRN This change looks good =) Can you go ahead and sign the CLA for me so I can get it merged? |
@BeksOmega Can you tell me how to sign CLA, as i'm new to this term. |
Check out the message from above @AbhinavKRN =)
|
@BeksOmega Done with CLA! |
@BeksOmega there was some error with prettier, i have resolved it again. Can you check it. |
Thank you again for the work on this @AbhinavKRN ! |
The basics
The details
Resolves
Fixes #8396
Proposed Changes
This pull request addresses a RangeError: Maximum call stack size exceeded error that occurs when users attempt to drag a block with multiple fingers on a workspace where zoom controls/wheel are disabled. The change modifies the handling of touch events to prevent an infinite loop.
Reason for Changes
The error occurs due to the interaction between the
handleMove
andhandleTouchMove
functions. If a multi-touch event does not meet the conditions for a pinch zoom, the functions call each other recursively, resulting in a maximum call stack size exceeded error. This issue has caused disruptions, triggering alarms in our monitoring system and impacting our on-call engineers.Test Coverage
I have tested the changes manually by:
The RangeError no longer occurs, and the drag operation works as expected. Additional unit tests have been created to simulate multi-touch events and verify that no infinite loop occurs.
Documentation
No documentation updates are required as this change only affects internal event handling.
Additional Information
The issue was detected using NewRelic, which reported an increase in JS errors since migrating additional labs to Google Blockly. The proposed solution involves modifying the
handleTouchMove
function to handle the move operation directly when pinch zoom conditions are not met, preventing the recursive call loop.Detailed Analysis
The problem occurs in the interaction between
handleMove
andhandleTouchMove
:handleMove
Function: