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.

Sign up for GitHub

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

Jump to bottom

Aborting the closing of read-only pane upon connection exit puts the pane in invalid state #9572

Closed
Don-Vito opened this issue Mar 21, 2021 · 1 comment
Closed

Aborting the closing of read-only pane upon connection exit puts the pane in invalid state #9572

Don-Vito opened this issue Mar 21, 2021 · 1 comment
Labels
Needs-Tag-Fix Doesn't match tag requirements Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.

Comments

@Don-Vito
Copy link
Contributor

Windows Terminal version (or Windows build number)

Dev - latest

Other Software

No response

Steps to reproduce

  • Run ```Sleep 10; exit``
  • Make the pane read-only
  • Upon read-only dialog appearance - dismiss it

Expected Behavior

The pane remains open with no active connection in it

Actual Behavior

Tab is removed from MRU (and gets unzoomed if it was previously zoomed)

The problem is related to the fact that all CloseHandlers are invoked by the pane, even if the handler in the Terminal Page has aborted the closing.
@ghost ghost added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels Mar 21, 2021
This was referenced Mar 21, 2021
Merged
Merged
@ghost ghost added the In-PR This issue has a related PR label Mar 21, 2021
ghost pushed a commit that referenced this issue Mar 22, 2021
@Don-Vito
Prevent tab context menu from closing root pane directly (#9571)
a5ff745 
## Summary of the Pull Request
Currently a repeated attempt to close a read-only tab from context menu,
will bring the terminal into invalid state if user dismisses close action.

There are two root causes for this:
1. The tab close menu triggers the closing of the root pane
(rather than invoking close tab flow in the Terminal Page).
2. Currently panes are not aware that the closing was canceled,
and thus they trigger the Closed event, putting the system in a weird state,
where the Closed handlers were invoked, but the Pane remains.

This PR mitigates #9502, by addressing the first root cause
(the fix is trivial and hopefully can be serviced).
Moreover, it addresses the only existing UI flow that can trigger the issue.

The remaining problematic flow will occur when the connection is closed.
I have created a separate Issue to track it: 
#9572
as I guess the PR for it might be more complex.

<!-- Please review the items on the PR checklist before submitting-->
## PR Checklist
* [x] Closes #9502
* [x] CLA signed. 
* [ ] Tests added/passed
* [ ] Documentation updated. 
* [ ] Schema updated.
* [ ] I've discussed this with core contributors already.
@zadjii-msft zadjii-msft removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Mar 26, 2021
@ghost ghost added Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. and removed In-PR This issue has a related PR labels Mar 30, 2021
@ghost ghost closed this as completed in c585a93 Mar 30, 2021
DHowett pushed a commit that referenced this issue Apr 2, 2021
@Don-Vito @DHowett
Prevent tab context menu from closing root pane directly (#9571)
9e49ff2 
## Summary of the Pull Request
Currently a repeated attempt to close a read-only tab from context menu,
will bring the terminal into invalid state if user dismisses close action.

There are two root causes for this:
1. The tab close menu triggers the closing of the root pane
(rather than invoking close tab flow in the Terminal Page).
2. Currently panes are not aware that the closing was canceled,
and thus they trigger the Closed event, putting the system in a weird state,
where the Closed handlers were invoked, but the Pane remains.

This PR mitigates #9502, by addressing the first root cause
(the fix is trivial and hopefully can be serviced).
Moreover, it addresses the only existing UI flow that can trigger the issue.

The remaining problematic flow will occur when the connection is closed.
I have created a separate Issue to track it:
#9572
as I guess the PR for it might be more complex.

<!-- Please review the items on the PR checklist before submitting-->
## PR Checklist
* [x] Closes #9502
* [x] CLA signed.
* [ ] Tests added/passed
* [ ] Documentation updated.
* [ ] Schema updated.
* [ ] I've discussed this with core contributors already.

(cherry picked from commit a5ff745)
@ghost
Copy link

ghost commented Apr 14, 2021

🎉This issue was addressed in #9574, which has now been successfully released as Windows Terminal Preview v1.8.1032.0.:tada:

Handy links:

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Needs-Tag-Fix Doesn't match tag requirements Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@Don-Vito @zadjii-msft