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

Window layout persisted in state.json even if window persistence not enabled #17207

Closed
ianjoneill opened this issue May 8, 2024 · 4 comments · Fixed by #17211
Closed

Window layout persisted in state.json even if window persistence not enabled #17207

ianjoneill opened this issue May 8, 2024 · 4 comments · Fixed by #17211
Labels
Area-Settings Issues related to settings and customizability, for console or terminal 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 Priority-1 A description (P1) Product-Terminal The new Windows Terminal.

Comments

@ianjoneill
Copy link
Contributor

Windows Terminal version

1.20.11215.0

Windows build number

10.0.22631.3447

Other Software

No response

Steps to reproduce

  1. Ensure window persistence is turned off
  2. Set windows terminal as your default terminal application
  3. Go to run and type cmd /k echo hello world
  4. Close the terminal
  5. Inspect %LOCALAPPDATA%\Packages\Microsoft.WindowsTerminalPreview_8wekyb3d8bbwe\LocalState\state.json

Expected Behavior

There is no history of the command that I ran in state.json.

Actual Behavior

state.json has leaked my super secret command line invocation!

{
  "persistedWindowLayouts" : 
  [
    {
      "initialPosition" : "234,234",
      "initialSize" : 
      {
        "height" : 436.0,
        "width" : 873.0
      },
      "launchMode" : "default",
      "tabLayout" : 
      [
        {
          "action" : "newTab",
          "commandline" : "\"C:\\WINDOWS\\system32\\cmd.exe\" /k echo hello world",
          "profile" : "Command Prompt",
          "startingDirectory" : "C:\\Users\\Ian",
          "suppressApplicationTitle" : false,
          "tabTitle" : "C:\\WINDOWS\\system32\\cmd.exe"
        }
      ]
    }
  ],
  "settingsHash" : "190f05f1cd1315b4-01d98faa8aecd332"
}

This is not reproducible on version 1.19.11213.0.

@ianjoneill ianjoneill 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 May 8, 2024
@lhecker
Copy link
Member

lhecker commented May 8, 2024

Oh dear... But #17206 was created a little earlier than this issue, so lets close it in favor of the older one. 🙂 /dup #17206

Copy link
Contributor

Hi! We've identified this issue as a duplicate of another one that already exists on this Issue Tracker. This specific instance is being closed in favor of tracking the concern over on the referenced thread. Thanks for your report!

@microsoft-github-policy-service microsoft-github-policy-service bot added Resolution-Duplicate There's another issue on the tracker that's pretty much the same thing. and removed Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels May 8, 2024
@ianjoneill
Copy link
Contributor Author

I figured they were related, but not quite the same, so opened a separate ticket - given that this pre-dates 1.21.

@lhecker lhecker reopened this May 8, 2024
@lhecker
Copy link
Member

lhecker commented May 8, 2024

Oof, I saw the title, read the repro steps, and considering the timing I thought it would be a dupe. I'm sorry. 😣 However, it is in fact the exact same underlying reason: 1.20 contains the same session persistence improvements that 1.21 got, but without the buffer restore code. This is why both 1.20 and 1.21 have the same bug: They persist, even when they shouldn't.

I'll mark #17211 as fixing this issue. We'll then backport the parts that are relevant for 1.20 (a single if condition basically).

@lhecker lhecker added Area-Settings Issues related to settings and customizability, for console or terminal Product-Terminal The new Windows Terminal. In-PR This issue has a related PR Priority-1 A description (P1) and removed Resolution-Duplicate There's another issue on the tracker that's pretty much the same thing. labels May 8, 2024
github-merge-queue bot pushed a commit that referenced this issue May 8, 2024
This fixes 2 bugs:
* `PersistState` being called when the window is closed
  (as opposed to closing the tab). The settings check was missing.
* Session cleanup running depending on whether the feature is
  currently enabled as opposed to whether it was enabled on launch.

Closes #17206
Closes #17207

## Validation Steps Performed
* Create a bunch of leftover buffer_*.txt files by running
  the current Dev version off of main
* Build this branch, then open and close a window
* All buffer_*.txt are gone and state.json is cleaned up ✅
@microsoft-github-policy-service microsoft-github-policy-service bot added the Needs-Tag-Fix Doesn't match tag requirements label May 8, 2024
DHowett pushed a commit that referenced this issue May 13, 2024
This fixes 2 bugs:
* `PersistState` being called when the window is closed
  (as opposed to closing the tab). The settings check was missing.
* Session cleanup running depending on whether the feature is
  currently enabled as opposed to whether it was enabled on launch.

Closes #17206
Closes #17207

## Validation Steps Performed
* Create a bunch of leftover buffer_*.txt files by running
  the current Dev version off of main
* Build this branch, then open and close a window
* All buffer_*.txt are gone and state.json is cleaned up ✅

(cherry picked from commit dbac3a1)
Service-Card-Id: 92515454
Service-Version: 1.21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Settings Issues related to settings and customizability, for console or terminal 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 Priority-1 A description (P1) Product-Terminal The new Windows Terminal.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants