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

Multiple 'Settings' entries on title bar context menu. Crashes Terminal. #16211

Closed
strangebit opened this issue Oct 22, 2023 · 1 comment · Fixed by #16225
Closed

Multiple 'Settings' entries on title bar context menu. Crashes Terminal. #16211

strangebit opened this issue Oct 22, 2023 · 1 comment · Fixed by #16225
Assignees
Labels
Area-Windowing Window frame, quake mode, tearout In-PR This issue has a related PR Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Tag-Fix Doesn't match tag requirements Product-Terminal The new Windows Terminal.

Comments

@strangebit
Copy link

Windows Terminal version

1.18.2822.0

Windows build number

10.0.19045.0

Other Software

No response

Steps to reproduce

Not sure what is triggering it unfortunately.

Expected Behavior

No response

Actual Behavior

I've noticed that sometimes, after using Terminal for some time, the right-click context menu on the titlebar ends up getting multiple Settings entries. When I click on one of them, Terminal crashes. When there's only one it doesn't crash; it opens the Settings panel as it should.

See the video below. Again this is after using Terminal for some time, opening and closing windows, etc. I don't know how to reproduce the bug consistently.

2023-10-22.03-01-01.mp4

I've seen it before with as many as six of these Settings entries.

The crash in the above video corresponds to this event (if it helps):

Faulting application name: WindowsTerminal.exe, version: 1.18.2310.9002, time stamp: 0x65248b0f
Faulting module name: TerminalApp.dll, version: 1.18.2310.9002, time stamp: 0x65248aa8
Exception code: 0xc0000005
Fault offset: 0x0000000000050c6a
Faulting process ID: 0x4d8c
Faulting application start time: 0x01da047ff2360b38
Faulting application path: C:\Program Files\WindowsApps\Microsoft.WindowsTerminal_1.18.2822.0_x64__8wekyb3d8bbwe\WindowsTerminal.exe
Faulting module path: C:\Program Files\WindowsApps\Microsoft.WindowsTerminal_1.18.2822.0_x64__8wekyb3d8bbwe\TerminalApp.dll
Report ID: 4ed86fa0-563b-4b67-bb0e-de7b5ea21887
Faulting package full name: Microsoft.WindowsTerminal_1.18.2822.0_x64__8wekyb3d8bbwe
Faulting package-relative application ID: App

I'm not sure if it's related to my other bug report #16100.

@strangebit strangebit added Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Oct 22, 2023
@zadjii-msft
Copy link
Member

I'm gonna guess this is also a window refrigeration bug.

I'd guess that the problem code is AppHost::_SystemMenuChangeRequested not getting called to remove the old entries before refrigeration.

Oh it definitely is. TerminalWindow only ever adds the settings entry when it gets created. AppHost however needs to clear that old entry out before we refrigerate the HWND. The old entries end up sticking around, which is why the old entries will explode the Terminal.

@zadjii-msft zadjii-msft added this to the Terminal v1.20 milestone Oct 24, 2023
@zadjii-msft zadjii-msft added Product-Terminal The new Windows Terminal. Area-Windowing Window frame, quake mode, tearout and removed Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Oct 24, 2023
zadjii-msft added a commit that referenced this issue Oct 24, 2023
As in the title. Also fixes a crash for refrigeration with the rainbow border.
Closes #16211

Tested by manually forcing us into Windows 10 mode (to refrigerate the window).
That immediately repros the bugn, which was simple enough to fix.
@microsoft-github-policy-service microsoft-github-policy-service bot added the In-PR This issue has a related PR label Oct 24, 2023
@zadjii-msft zadjii-msft self-assigned this Oct 24, 2023
DHowett pushed a commit that referenced this issue Oct 24, 2023
As in the title. Also fixes a crash for refrigeration with the rainbow
border.

Closes #16211

Tested by manually forcing us into Windows 10 mode (to refrigerate the
window). That immediately repros the bug, which was simple enough to
fix.
@microsoft-github-policy-service microsoft-github-policy-service bot added the Needs-Tag-Fix Doesn't match tag requirements label Oct 24, 2023
DHowett pushed a commit that referenced this issue Oct 26, 2023
As in the title. Also fixes a crash for refrigeration with the rainbow
border.

Closes #16211

Tested by manually forcing us into Windows 10 mode (to refrigerate the
window). That immediately repros the bug, which was simple enough to
fix.

(cherry picked from commit d8c7719)
Service-Card-Id: 90928408
Service-Version: 1.19
DHowett pushed a commit that referenced this issue Nov 7, 2023
As in the title. Also fixes a crash for refrigeration with the rainbow
border.

Closes #16211

Tested by manually forcing us into Windows 10 mode (to refrigerate the
window). That immediately repros the bug, which was simple enough to
fix.

(cherry picked from commit d8c7719)
Service-Card-Id: 90928407
Service-Version: 1.18
DHowett pushed a commit that referenced this issue Nov 7, 2023
As in the title. Also fixes a crash for refrigeration with the rainbow
border.

Closes #16211

Tested by manually forcing us into Windows 10 mode (to refrigerate the
window). That immediately repros the bug, which was simple enough to
fix.

(cherry picked from commit d8c7719)
Service-Card-Id: 90928407
Service-Version: 1.18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Windowing Window frame, quake mode, tearout In-PR This issue has a related PR Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Tag-Fix Doesn't match tag requirements Product-Terminal The new Windows Terminal.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants