Fix pane event handlers being unbound #17750
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
microsoft-github-policy-service
bot
added
Issue-Bug
DHowett
approved these changes
Aug 20, 2024
carlos-zamora
approved these changes
Aug 20, 2024
zadjii-msft
approved these changes
Aug 21, 2024
DHowett
pushed a commit
that referenced
this pull request
Aug 21, 2024
I don't know what has changed between #17450 and now, but that fix doesn't seem necessary anymore. If you add this action: ```json { "keys": "ctrl+a", "command": { "action": "splitPane", "commandline": "cmd /c exit" } } ``` and repeatedly spam Ctrl-A it used to lead to crashes. That doesn't happen anymore, because some other PR must've fixed that. Reverting #17450 fixes the issue found in #17578: Because the content pointer didn't get reset to null anymore it meant that the root pane retained the pointer after a split. After closing the split off pane, it would assign the remaining one back to the root, which would cause the still existing content pointer to be closed. That pointer is potentially the same as the remaining pane and so no close events would get received anymore. Closes #17578 ## Validation Steps Performed * Add the above action and spam it ✅ * Start with an empty window, split pane, type `exit` in the new pane then type it in the original pane. It closes the window ✅ (cherry picked from commit 056af83) Service-Card-Id: PVTI_lADOAF3p4s4AmhmszgSGCfE Service-Version: 1.21
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I don't know what has changed between #17450 and now, but that fix
doesn't seem necessary anymore. If you add this action:
{ "keys": "ctrl+a", "command": { "action": "splitPane", "commandline": "cmd /c exit" } }and repeatedly spam Ctrl-A it used to lead to crashes. That doesn't
happen anymore, because some other PR must've fixed that.
Reverting #17450 fixes the issue found in #17578: Because the content
pointer didn't get reset to null anymore it meant that the root
pane retained the pointer after a split. After closing the split off
pane, it would assign the remaining one back to the root, which would
cause the still existing content pointer to be closed. That pointer
is potentially the same as the remaining pane and so no close events
would get received anymore.
Closes #17578
Validation Steps Performed
exitin the new panethen type it in the original pane. It closes the window ✅