Process Model v3 / Tab tear-out ship list #14957

zadjii-msft · 2023-03-06T15:34:47Z

Moving this to separate from #14900. I want that to SPECIFICALLY track tag dragging UX issues (which could apply to us or maybe anyone else using TabView).

For general tab tear-out &U drag drop UX issues, see: ---> #14900 <---

This is a list of things we need to fix to get #5000 / #1256 out the door.

Original PRs:

PR Blockers

We shouldn't even merge these to main like this

PR bugs

Give feedback

I went into settings to turn on the tray icon, and it crashed when I hit Save. (fixed in 35c7474ab)
Another curious A/V crash when hot-reloading, with a null Dispatcher? (fixed in 6e4b2e104)
(ainulindale) Windows don't get activated when they're first created (fixed in 704220f7e)
(ainu) defterm is ALL SORTS of fucked (fixed in a1a86eb6c)
(party) tear-out of a single window (with no other running windows) closes the Terminal
(party) Tear-out a window upscales it on high DPI (805b880da)
(?) wt -w _quake is broken?
Options

Ship blockers

We can merge these, but iterate in post

_windowExitedHandler() would have to be called inside the try clause as a wil::scope_exit

see discussion in #14843 (comment)

Let's just do it in post

Did we fix it?

Things we may have accidentally fixed on the way?

Fixed?

Give feedback

Closing terminal that was launched from another terminal instance minimizes the parent terminal #14012

Area-DefApp Area-Windowing Issue-Bug Needs-Attention Needs-Triage Priority-2 Product-Terminal
Options

The text was updated successfully, but these errors were encountered:

ForbiddenEra · 2023-03-18T12:11:35Z

This is a feature I've been wanting for a while; I'd be happy to give it a test if possible and see if I run into any issues.. However, I'm not quite eager enough to do anything such as building the sources myself. If there's any sort of Beta/RC containing this feature, that'd be great - I already am running the Preview build however.

I do use quite a few screens with multiple dGPUs+iGPU, fancy-zones and such, not sure if my setup would be considered any more trying towards testing (eg. dragging tabs onto other screens, across different GPUs or dropping right into a fancyzone or something - I've seen issues w/other things like browsers handling this in the past though it seems like most of those issues have been solved in those apps)

Otherwise, I'll just thank everyone for their hard work and wait for it to drop and test it then and hopefully not have to report any issues.

:)

If we get initialized with a window name, this will be called before XAML is stood up, and constructing a PropertyChangedEventArgs will throw. So don't. Regressed in #14843 Related to #5000, #14957

zadjii-msft · 2023-03-31T19:05:43Z

Dustin's thing about content

Spawned from this discussion:
#14935 (comment)

Okay, I think I get it. If we can associate the Content directly (or the content tree) with the DataPackage, we no longer need to stash anything. We could probably also completely get rid of ContentIds and ContentManager (!).
Here's why I think that. It looks like you do always get a datapackage or a tab sender. Therefore, you can check the datapackage, read the source window, if it's the same don't detach anything. If it's different, ask the content tree to bubble a detach up to its original hosting window. detach remains driven recipient-side

Fixes a bug where you'd drag across the boundary and the new window would be at the wrong size related to #14957

zadjii-msft · 2023-04-24T16:58:37Z

MSFT:43995981 - the `_checkWindowsForNotificationIcon` one

Looking at a dump, the crash is due to a race.

A window [A] is created. It lives its life.
[A] starts dying.
- It dtors the AppHost
- It releases its Logic() member.
At this moment, a new Terminal process starts.
- this causes a ProposeCommandline in the emperor process.
- This causes the monarch to create a new peasant for the new window.
- This causes the monarch to raise its _WindowCreatedHandlers.
- This gets into WindowEmperor::_checkWindowsForNotificationIcon.
- This takes the _windows lock.
Thread A resumes.
- [A] falls out the bottom of RunMessagePump
- The cleanup triggers _windowExitedHandler
- It attempts to lock _windows to remove itself.
The RPC thread (that's creating the new peasant) resumes.
- It tries to get at the _windowThread->Logic(), but oh dear, the AppHost has already dtor'd.

…AppHost (#15231) See #14957 (comment). I think there's a race here that lets the WindowEmperor muck around with the window after it's done, but before we remove it from our list of threads. This _should_ remove the thread from the list, _then_ null out the AppHost, then flush the XAML queue, preventing the A/V. Closes MSFT:43995981

…5175) This bug causes AtlasEngine to render buffer contents with an incorrect `cellCount`, which may either cause it to draw the contents only partially, or potentially access the TextBuffer contents out of bounds. `EnablePainting` sets the `_viewport` to the current viewport for some unfortunate (and quite buggy/incorrect) caching purposes, which causes `_CheckViewportAndScroll()` to think that the viewport hasn't changed in the new window. We can ensure `_CheckViewportAndScroll()` works by also setting `_forceUpdateViewport` to `true`. Part of #14957 ## PR Checklist * Tear out a tab from a smaller window to a larger window * Renderer contents adept to the larger window size ✅ --------- Co-authored-by: Mike Griese <[email protected]> Co-authored-by: Dustin L. Howett <[email protected]>

We had a report in a mail thread that someone's Terminal windows were getting created hidden, and never showing themselves. As a theory, I'm guessing that dwFlags didn't say that we should actually use `wShowWindow`. So, to be more correct, let's actually obey that. I'm gonna send this package to them to see if it fixes them. Related to #14957. Likely regressed in #13838.

This was in pursuit of #15156. I need an ack from OP to make sure this is good enough. Related to #14957

If you were really fast, and closed one window, and then tried to drag the only tab out of the last remaining window, the Terminal could explode. It'd attempt to restore the previous window state, and explode. Easy way to stop this (also, be more robust): just don't attempt to restore windows during tear-out. That's obvious. This is a part of #14957

This was in pursuit of #15156. I need an ack from OP to make sure this is good enough. Related to #14957

This was removed in #14843, but the velocity flag wasn't. Related to #14957

As on the tin. Blocking for 1.18. Tracked in #14957

[Just don't go](https://youtu.be/jmIFwSKnuRM?t=102) Tracked in #14957

As on the tin. Blocking for 1.18. Tracked in #14957

Before process model v3, each Terminal window was running in its own process, each with its own CWD. This allowed `startingDirectory: .` to work relative to the terminal's own CWD. However, now all windows share the same process, so there can only be one CWD. That's not great - folks who right-click "open in terminal", then "Use parent process directory" are only ever going to be able to use the CWD of the _first_ terminal opened. This PR remedies this issue, with a theory we had for another issue. Essentially, we'll give each Terminal window a "virtual" CWD. The Terminal isn't actually in that CWD, the terminal is in `system32`. This also will prevent the Terminal from locking the directory it was originally opened in. * Closes #5506 * There wasn't a 1.18 issue for "Use parent process directory is broken" yet, presumably selfhosters aren't using that feature * Related to #14957 Many more notes on this topic in #4637 (comment) > **Warning** > ## Breaking change‼️ This will break a profile like ```json { "commandline": "media-test.exe", "name": "Use CWD for media-test", "startingDirectory": "." }, ``` if the user right-clicks "open in terminal", then attempts to open that profile. There's some theoretical work we could do in a follow up to fix this, but I'm inclined to say that relative paths for `commandline`s were already dangerous and should have been avoided.

Before process model v3, each Terminal window was running in its own process, each with its own CWD. This allowed `startingDirectory: .` to work relative to the terminal's own CWD. However, now all windows share the same process, so there can only be one CWD. That's not great - folks who right-click "open in terminal", then "Use parent process directory" are only ever going to be able to use the CWD of the _first_ terminal opened. This PR remedies this issue, with a theory we had for another issue. Essentially, we'll give each Terminal window a "virtual" CWD. The Terminal isn't actually in that CWD, the terminal is in `system32`. This also will prevent the Terminal from locking the directory it was originally opened in. * Closes #5506 * There wasn't a 1.18 issue for "Use parent process directory is broken" yet, presumably selfhosters aren't using that feature * Related to #14957 Many more notes on this topic in #4637 (comment) > **Warning** > ## Breaking change‼️ This will break a profile like ```json { "commandline": "media-test.exe", "name": "Use CWD for media-test", "startingDirectory": "." }, ``` if the user right-clicks "open in terminal", then attempts to open that profile. There's some theoretical work we could do in a follow up to fix this, but I'm inclined to say that relative paths for `commandline`s were already dangerous and should have been avoided. (cherry picked from commit 5c08a86) Service-Card-Id: 89180224 Service-Version: 1.18

Don't go. Tracked in #14957

Don't go. Tracked in #14957 (cherry picked from commit 1324a01) Service-Card-Id: 89180217 Service-Version: 1.18

ForbiddenEra · 2023-06-16T10:39:59Z

omgomgomg I just tried tab tear out after a recent update andddddddddd it works!!!!!! finally! so glad to have this feature.

<3 <3 for your hard works!!

microsoft-github-policy-service bot 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 6, 2023

zadjii-msft mentioned this issue Mar 6, 2023

[MEGATHREAD] Tab drag/drop/tear-out gaps #14900

Open

19 tasks

This comment was marked as resolved.

Sign in to view

microsoft-github-policy-service bot removed the Needs-Tag-Fix Doesn't match tag requirements label Mar 7, 2023

zadjii-msft added this to the Terminal v1.18 milestone Mar 7, 2023

microsoft-github-policy-service bot removed Needs-Tag-Fix Doesn't match tag requirements labels Mar 7, 2023

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 7, 2023

zadjii-msft self-assigned this Mar 7, 2023

This comment was marked as resolved.

Sign in to view

zadjii-msft mentioned this issue Mar 22, 2023

Fix wt -w _quake by not throwing when setting the window name #15030

Merged

SubsTheTechnomancer mentioned this issue Mar 23, 2023

Multiple Quake Windows can be opened #15037

Open

zadjii-msft mentioned this issue Apr 3, 2023

Use DIPs for the window bounds when tearing out #15094

Merged

zadjii-msft added a commit that referenced this issue Apr 4, 2023

Use DIPs for the window bounds when tearing out (#15094)

c0f1456

Fixes a bug where you'd drag across the boundary and the new window would be at the wrong size related to #14957

This comment was marked as resolved.

Sign in to view

zadjii-msft mentioned this issue Apr 24, 2023

Remove the window thread from the list of threads before nulling the AppHost #15231

Merged

zadjii-msft added a commit that referenced this issue Apr 28, 2023

Add default bindings for "move tab/pane to a new window"

3f42aeb

This was in pursuit of #15156. I need an ack from OP to make sure this is good enough. Related to #14957

zadjii-msft mentioned this issue Apr 28, 2023

Add default bindings for "move tab/pane to a new window" #15258

Merged

zadjii-msft mentioned this issue Apr 28, 2023

Don't explode when we tear out the last tab of the window #15259

Merged

microsoft-github-policy-service bot pushed a commit that referenced this issue May 1, 2023

Add default bindings for "move tab/pane to a new window" (#15258)

1da6131

This was in pursuit of #15156. I need an ack from OP to make sure this is good enough. Related to #14957

This was referenced May 3, 2023

Use a "virtual CWD" for each terminal window #15280

Merged

Remove the IsolatedMonarchMode velocity flag #15300

Merged

DHowett pushed a commit that referenced this issue May 8, 2023

Remove the IsolatedMonarchMode velocity flag (#15300)

c18a4fe

This was removed in #14843, but the velocity flag wasn't. Related to #14957

zadjii-msft added a commit that referenced this issue May 10, 2023

Don't let a window be created with the literal name "new"

98d2d32

As on the tin. Blocking for 1.18. Tracked in #14957

zadjii-msft mentioned this issue May 10, 2023

Don't let a window be created with the literal name "new" #15323

Merged

zadjii-msft added a commit that referenced this issue May 10, 2023

Don't even attempt to MoveContent to the same window you started in

e714c59

[Just don't go](https://youtu.be/jmIFwSKnuRM?t=102) Tracked in #14957

zadjii-msft mentioned this issue May 10, 2023

Don't even try to MoveContent to the same window you started in #15325

Merged

microsoft-github-policy-service bot pushed a commit that referenced this issue May 11, 2023

Don't let a window be created with the literal name "new" (#15323)

c553b21

As on the tin. Blocking for 1.18. Tracked in #14957

zadjii-msft modified the milestones: Terminal v1.18, Terminal v1.19 May 12, 2023

zadjii-msft removed their assignment May 12, 2023

DHowett pushed a commit that referenced this issue May 12, 2023

Don't even try to MoveContent to the same window you started in (#15325)

1324a01

Don't go. Tracked in #14957

DHowett pushed a commit that referenced this issue May 12, 2023

Don't even try to MoveContent to the same window you started in (#15325)

11233c5

Don't go. Tracked in #14957 (cherry picked from commit 1324a01) Service-Card-Id: 89180217 Service-Version: 1.18

zadjii-msft modified the milestones: Terminal v1.19, Backlog Oct 4, 2023

zadjii-msft mentioned this issue Oct 29, 2024

Lead the user to the AI settings when no provider is set up #18121

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Process Model v3 / Tab tear-out ship list #14957

Process Model v3 / Tab tear-out ship list #14957

zadjii-msft commented Mar 6, 2023 •

edited

Loading

PR bugs

Follow-ups

In post

Fixed?

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

ForbiddenEra commented Mar 18, 2023

zadjii-msft commented Mar 31, 2023

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

zadjii-msft commented Apr 24, 2023

ForbiddenEra commented Jun 16, 2023

Process Model v3 / Tab tear-out ship list #14957

Process Model v3 / Tab tear-out ship list #14957

Comments

zadjii-msft commented Mar 6, 2023 • edited Loading

For general tab tear-out &U drag drop UX issues, see: ---> #14900 <---

Original PRs:

PR Blockers

PR bugs

Ship blockers

Follow-ups

Let's just do it in post

In post

Did we fix it?

Fixed?

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

ForbiddenEra commented Mar 18, 2023

zadjii-msft commented Mar 31, 2023

Dustin's thing about content

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

zadjii-msft commented Apr 24, 2023

MSFT:43995981 - the _checkWindowsForNotificationIcon one

ForbiddenEra commented Jun 16, 2023

zadjii-msft commented Mar 6, 2023 •

edited

Loading

MSFT:43995981 - the `_checkWindowsForNotificationIcon` one