## Summary of the Pull Request
When a user had "Disable Scroll Forward" enabled and switched to the alt buffer and maximized the console, then restored down, we'd crash. Now we don't.
## References
## PR Checklist
* [x] Closes#1206
* [x] I work here
* [x] Tests added/passed
## Detailed Description of the Pull Request / Additional comments
The problem is that we'd previously try to "anchor" the viewport to the virtual bottom when resizing like this. This would also cause us to move the top of the viewport down, into the buffer. However, if the alt buffer is getting smaller, we don't want to do this - if we anchor to the old _virtualBottom, the bottom of the viewport will actually be outside the current buffer.
This could theoretically happen with the main buffer too, but it's much easier to repro with the alt buffer.
* change 1: add settings pointer and some member variables to page
* clean up the boundary between Page and App - First working version
* First CR review change
* Sync and remove declaration of TraceLogger provider
* Code review round 2 - apply missed new changes
* remove useless comment
* CR change round 3
* CR minor changes
* apply changes from Aug 6th to Aug 14th
* Code review changes round 4
* Apply changes on Aug 16
* Cr changes on 8/20
* CR changes on 8-26
* correct syncing mistakes and fix formatting issues
* CR changes on 8-29
* CR changes 9-4
* apply new changes of App
* Format fix
This pull request introduces a copy of the code from kernel32.dll that
implements CreatePseudoConsole, ClosePseudoConsole and
ResizePseudoConsole. Apart from some light modifications to fit into the
infrastructure in this project and support launching OpenConsole.exe, it
is intended to be 1:1 with the code that ships in Windows.
Any guideline violations in this code are likely intentional. Since this
was built into kernel32, it uses the STL only _very sparingly._
Consumers of this library must make sure that conpty.lib lives earlier
in the link line than onecoreuap_apiset, onecoreuap, onecore_apiset,
onecore or kernel32.
Refs #1130.
[Git2Git] Git Train: Merge of building/rs_onecore_dep_uxp/190820-1847 into official/rs_onecore_dep_uxp Retrieved from https://microsoft.visualstudio.com os OS official/rs_onecore_dep_uxp 73e964d4046c37df3030970cae1ae32e83103fb5
(cherry picked from commit 8c63dff982093db1af7e2bb46b49af884dfec0c5)
* Merge pane splitting methods
Having separate Horizontal/Vertical versions made it hard to manage, and App.cpp already made use of Pane::SplitState so it made sense to have that be the descriminator
* Rename Tab::(Can)AddSplit to (Can)SplitPane to align with Pane methods
Split was used as a noun in Tab but a verb in Pane, which felt odd
* Remove unused local variable in Pane::_CanSplit
* Remove redundant 'else' branches in Pane
Improves readibility for all 'low hanging fruit' cases where the 'if' was returning.
When the scrollback buffer is empty, the RIS escape sequence (Reset to Initial
State) will fail to clear the screen, or reset any of the state. And when there
is something in the scrollback, it doesn't get cleared completely, and the
screen may get filled with the wrong background color (it should use the
default color, but it actually uses the previously active background color).
This commit attempts to fix those issues.
The initial failure is caused by the `SCREEN_INFORMATION::WriteRect` method
throwing an exception when passed an empty viewport. And the reason it's passed
an empty viewport is because that's what the `Viewport::Subtract` method
returns when the result of the subtraction is nothing. The PR fixes the
problem by making the `Viewport::Subtract` method actually return nothing in
that situation.
This is a change in the defined behavior that also required the associated
viewport tests to be updated. However, it does seem a sensible change, since
the `Subtract` method never returns empty viewports under any other
circumstances. And the only place the method seems to be used is in the
`ScrollRegion` implementation, where the previous behavior is guaranteed to
throw an exception.
The other issues are fixed simply by changing the order in which things are
reset in the `AdaptDispatch::HardReset` method. The call to `SoftReset` needed
to be made first, so that the SGR attributes would be reset before the screen
was cleared, thus making sure that the default background color would be used.
And the screen needed to be cleared before the scrollback was erased, otherwise
the last view of the screen would be retained in the scrollback buffer.
These changes also required existing adapter tests to be updated, but not
because of a change in the expected behaviour. It's just that certain tests
relied on the `SoftReset` happening later in the order, so weren't expecting it
to be called if say the scrollback erase had failed. It doesn't seem like the
tests were deliberately trying to verify that the SoftReset _hadn't_ been
called.
In addition to the updates to existing tests, this PR also add a new screen
buffer test which verifies the display and scrollback are correctly cleared
under the conditions that were previously failing.
Fixes#2307.
On occasion, in certain delegated access scenarios, we'll fail to read
the name of one or more of the user's Azure tenants. We would summarily
explode (because we're being strict about our incoming JSON, and we
didn't know that this was possible.)
Now we'll substitute in an alternate name and present the ID.
Fixes#2249.
* Update src/cascadia/TerminalConnection/AzureConnection.cpp
When we change the client ID, we're going to need to force people to log
in again.
We can do that either by:
1. Trying to log in and refresh the user's token and failing (displaying
a cryptic message like "you aren't on the internet, please get on the
internet"), **OR** by...
2. Getting out ahead of it, detecting when we would have failed for client
ID (and other) reasons, and _not trying at all._
This is option 2.
Refactors the accessibility providers (ScreenInfoUiaProvider and UiaTextRange) into a better separated model between ConHost and Windows Terminal.
ScreenInfoUiaProviderBase and UiaTextRangeBase are introduced. ConHost and Windows Terminal implement their own versions of ScreenInfoUiaProvider and UiaTextRange that inherit from their respective base classes.
WindowsTerminal's ScreenInfoUiaProvider --> TermControlUiaProvider
Since we're rendering with antialiasing enabled, we need to make sure
we're stroking actual pixels; to do that, we need to adjust all of our
coordinates by the StrokeWidth / 2. We're always using a stroke width of
1, so that means 0.5.
While I was here, I took the opportunity to fix the color of the grid
lines. Fixes#543.
This is more trouble than it's worth. We had code before to re-serialize
settings when they changed, to try and gracefully migrate settings from old
schemas to new ones. This is good in theory, but with #754 coming soon, this
is going to become a minefield. In the future we'll just always be providing a
base schema that's reasonable, so this won't matter so much. Keys that users
have that aren't understood will just be ignored, and that's _fine_.
Fixes a crash that can occur when splitting pane that was so small that the target panes would have a width/height of 0, causing DxRenderer to fail when creating the device resources.
This PR prevents both the call to `App::AddHorizontal/VerticalSplit` and the creation of the `TermControl` if the split would fail.
Closes#2401
## Details
`App::_SplitPane` calls `focusedTab->CanAddHorizontalSplit/CanAddHorizontalSplit` before it initializes the `TermControl` to avoid having to deal with the cleanup. If a split cannot occur, it will simply return.
**Question: Should we beep or something here?**
It then follows the same naming/flow style as the split operation, so: `Tab::CanAddHorizontalSplit -> Pane::CanSplitHorizontal ->Pane::_CanSplit`. The public pane methods will handle leaf/child the same as the current Split methods.
`_CanSplit` reuses existing logic like `_root.GetActualWidth/Height`, `Pane::_GetMinSize`, and the `Half` constant.
## Validation Steps Performed
1. Open a new tab
2. Attempt to split horizontally/vertically more than 6-8 times
Success: Pane will will eventually stop splitting rather than crashing the process.
* Move Clipboard::GenHTML to TextBuffer (add params)
Refactor RetrieveSelectedTextFromBuffer
Modify CopyToClipboardEventArgs to include HTML data
* minor code format fix
* PR Changes
NOTE: refactoring text buffer code is a separate task. New issue to be created.
* Refactor TextBuffer::GenHTML (#2038)
Fixes#1846.
* nit change
* x86 build fix
* nit changes
This commit also transitions our keybinding events and event handlers to a
TypedEventHandler model with an "event args" class, as specified in the
keybinding arguments specification (#1349). In short, every event can be marked
Handled independently, and a Handled event will stop bubbling out to the
terminal. An unhandled event will be passed off to the terminal as a standard
keypress.
This unifies our keybinding event model and provides a convenient place for
binding arguments to live.
Fixes#2285.
Related to #1349, #1142.
* Warn the user when their settings are bad
The start of work on #1348
* Display an error dialog for errors during validation
* Polish for PR
* Add a ton of tests
* Polish the _GetMessageText bits
* Add code to check for duplicate profiles
* Verify that many warnings work at the same time
* comments y'all
* Apply fixes for dustin's thoughts from PR
* Add a proper exception type, use an array instead of a map
* PR Fixes
* Fix x86 build break
* Add a bit on "using the defaults" when we encountering an exception
* remove a redundant variable
* guid->GUID
* Address Michael's PR comments
* Clean up this error text, and catch exceptions better
* Update src/cascadia/TerminalApp/Resources/en-US/Resources.resw
This commit replaces CodepointWidthDetector's
dynamically-generated map with a static constexpr one that's compiled
into the binary.
It also almost totally removes the notion of an `Invalid` width. We
definitely had gaps in our character coverage where we'd report a
character as invalid, but we'd then flatten that down to `Narrow` when
asked. By combining the not-present state and the narrow state, we get
to save a significant chunk of data.
I've tested this by feeding it all 0x10FFFF codepoints (and then some)
and making sure they 100% match the old code's outputs.
|------------------------------|---------------|----------------|
| Metric | Then | Now |
|------------------------------|---------------|----------------|
| disk space | 56k (`.text`) | 3k (`.rdata`) |
| runtime memory (allocations) | 1088 | 0 |
| runtime memory (bytes) | 51k | ~0 |
| memory behavior | not shared | fully shared |
| lookup time | ~31ns | ~9ns |
| first hit penalty | ~170000ns | 0ns |
| lines of code | 1088 | 285 |
| clarity | extreme | slightly worse |
|------------------------------|---------------|----------------|
I also took a moment and cleaned up a stray boolean that we didn't need.
Double/Triple click create a selection expanding beyond one cell. This PR makes it so that when you're dragging your mouse to expand the selection, you expand to the next delimiter defined by double/triple click.
So, double click expands by doubleClickDelimiter ranges. Triple click expands by line.
When you double/triple click, a word/line is selected. When you drag, that word/line will remain selected after the expansion occurs.
Closes#1933
## Details
Rather than resizing the selection when the mouse event occurs, I figured I'd do what I did with wide glyph selection: expand at render time.
We needed an enum `multiClickSelectionMode` to keep track of which expansion mode we're in.
Minor modifications to `_ExpandDoubleClickSelection*(COORD)` had to be made so that we can re-use them.
Actual expansion occurs in `_GetSelectionRects()`
## Validation Steps Performed
- generic double click test
- `dir` or `ls`
- double click a word
- drag up
- Works! ✔
- double click on delimiter test
- `dir` or `ls`
- double click a word delimiter (i.e.: space between words)
- drag up
- Works! ✔
- generic triple click test
- `dir` or `ls`
- triple click a line
- drag up
- Works! ✔
- ALT + double click test
- `dir` or `ls`
- hold ALT
- double click a word
- drag up
- Works! ✔
repeat above tests in following scenarios:
- when at top of scrollback
- drag down instead of up
### User Stories:
1. A user wants to be able to use the executable path as their starting title
- Does anyone want this?
2. A user wants to be able to set a custom starting title, but have that title be overridable
3. A user wants to be able to set an overridable starting title, different from the profile name
- Presumably someone will want this
4. A user totally wants to ignore the VT title and use something else
- This will make more sense in the post [#1320] "Support runtime variables in the custom user title" settings
### Solutions:
1. `name`, `startingTitle`, `tabTitle`
* a. `name` is only ever used as the profile name.
* b. If `startingTitle` isn't set, then the executable path is used
* c. If `startingTitle` is set, it's used as the initial title
* d. If `tabTitle` is set, it overrides the title from the terminal
* e. Current users of `tabTitle` need to manually update to the new behavior.
2. `name` as starting title, `tabTitle` as a different starting title
* a. `name` is used as the starting title and the profile name in the dropdown
* b. If `tabTitle` is set, we'll use that as the overridable starting title instead.
* c. In the future, `dynamicTabTitle` or `tabTitleOverride` could be added to support [#1320]
* d. Current users of `tabTitle` automatically get the new (different!) behavior.
* e. User Story 1 is impossible
- Does anyone want the behavior _ever_? Perhaps making that scenario impossible is good?
3. `name` unchanged, `tabTitle` as the starting title
* a. `name` is only ever used as the profile name.
* b. If `tabTitle` is set, we'll use that as the overridable starting title.
* c. In the future, `dynamicTabTitle` or `tabTitleOverride` could be added to support [#1320]
* d. Current users of `tabTitle` automatically get the new (different!) behavior.
4. `name` as starting title, `tabTitle` as different starting title, `suppressApplicationTitle` Boolean to force it to override
* a. `name`, `tabTitle` work as in Solution 2.
* b. When someone wants to be able to statically totally override that title (story 4), they can use `suppressApplicationTitle`
* c. `suppressApplicationTitle` name is WIP
* d. We'll add `suppressApplicationTitle` when someone complains
* e. If you really want story 1, use `tabTitle: c:\path\to\foo.exe` and `suppressApplicationTitle`.
[#1320]: https://github.com/microsoft/terminal/issues/1320
We've decided to pursue path 4.
In #1164 we learned that our CI doesn't support WinRT testing. This made us all sad. Since that merged, we haven't really added any TerminalApp tests, because it's a little too hard. You'd have to uncomment the entire file, and if the list of types changed you'd have to manually update the sxs manifest and appxmanifest.
Since that was all insane, I created a new Terminal App unittesting project without those problems.
1. The project is not named *Unit*Test*, so the CI won't run it, but it will run locally.
2. The project will auto-generate its SxS manifest, using the work from #1987.
3. We'll use the SxS manifest from step 2 to generate an AppxManifest for running packaged tests.
* This is the start of me trying to enable local unittesting again
* We've got a new unittests project that isn't named *unit*test*
* We're manually generating the SxS manifest for it. B/C we need to use it at runtime, we need to manually combine it into one manifest file
* the runas:UAP thing still doesn't work. We'll investigate.
* This shockingly works
but I'm still stuck with:
```
Summary of Errors Outside of Tests:
Error: TAEF: [HRESULT: 0x80270254] Failed to create the test host process for
out of process test execution. (The
IApplicationActivationManager::ActivateApplication call failed while using a
default host. TAEF's ETW logs which are gathered with the /enableEtwLogging
switch should contain events from relevant providers that may help to diagnose
the failure.)
```
* Cleaning this all up for review.
Frankly just pushing to see if it'll work in CI
* Couple things I noticed in the diff from master
* Apply @dhowett-msft's suggestions from code review
* Stop Roaming settings
Also migrate existing settings from RoamingState to LocalState.
Fixes#1770.
* * de-dupe these functions
* const a pair of things
* This should be in the previous commit
* use `unique_hfile`'s
* Make some of these wil things cleaner
* - moving string parameter into data member instead of copying it.
- removing noexcept from methods where an exception could be raised.
If std::terminate() call is desired instead, I guess those should be
left and std::move_if_noexcept() used to document the fact that it's
on purpose.
- std::moving local variable into argument when possible.
- change maxversiontested XML element to maxVersionTested.
- used of gsl::narrow_cast where appropriate to prevent warnings.
- fixed bug in TerminalSettings::SetColorTableEntry()
Fixes#1844
* Cleanup PCHs as the build rolls along to leave enough space on CI agents.
* Attempt to restrict pch cleanup to only CI agents.
* Write message when objects are deleted.
This moves the detection of AltGr keypresses in front of the shortcut
handling. This allows one to have Ctrl+Alt shortcuts, while
simultaneously being able to use the AltGr key for special characters.
The default azure connector profile only shows up if a) its a release build and b) its non-ARM64
Co-Authored-By: Dustin L. Howett (MSFT) <duhowett@microsoft.com>
Builds on the work of #1691 and #1915
Let's start with the easy change:
- `TermControl`'s `controlRoot` was removed. `TermControl` is a `UserControl`
now.
Ok. Now we've got a story to tell here....
### TermControlAP - the Automation Peer
Here's an in-depth guide on custom automation peers:
https://docs.microsoft.com/en-us/windows/uwp/design/accessibility/custom-automation-peers
We have a custom XAML element (TermControl). So XAML can't really hold our
hands and determine an accessible behavior for us. So this automation peer is
responsible for enabling that interaction.
We made it a FrameworkElementAutomationPeer to get as much accessibility as
possible from it just being a XAML element (i.e.: where are we on the screen?
what are my dimensions?). This is recommended. Any functions with "Core" at the
end, are overwritten here to tweak this automation peer into what we really
need.
But what kind of interactions can a user expect from this XAML element?
Introducing ControlPatterns! There's a ton of interfaces that just define "what
can I do". Thankfully, we already know that we're supposed to be
`ScreenInfoUiaProvider` and that was an `ITextProvider`, so let's just make the
TermControlAP an `ITextProvider` too.
So now we have a way to define what accessible actions can be performed on us,
but what should those actions do? Well let's just use the automation providers
from ConHost that are now in a shared space! (Note: this is a great place to
stop and get some coffee. We're about to hop into the .cpp file in the next
section)
### Wrapping our shared Automation Providers
Unfortunately, we can't just use the automation providers from ConHost. Or, at
least not just hook them up as easily as we wish. ConHost's UIA Providers were
written using UIAutomationCore and ITextRangeProiuder. XAML's interfaces
ITextProvider and ITextRangeProvider are lined up to be exactly the same.
So we need to wrap our ConHost UIA Providers (UIAutomationCore) with the XAML
ones. We had two providers, so that means we have two wrappers.
#### TermControlAP (XAML) <----> ScreenInfoUiaProvider (UIAutomationCore)
Each of the functions in the pragma region `ITextProvider` for
TermControlAP.cpp is just wrapping what we do in `ScreenInfoUiaProvider`, and
returning an acceptable version of it.
Most of `ScreenInfoUiaProvider`'s functions return `UiaTextRange`s. So we need
to wrap that too. That's this next section...
#### XamlUiaTextRange (XAML) <----> UiaTextRange (UIAutomationCore)
Same idea. We're wrapping everything that we could do with `UiaTextRange` and
putting it inside of `XamlUiaTextRange`.
### Additional changes to `UiaTextRange` and `ScreenInfoUiaProvider`
If you don't know what I just said, please read this background:
- #1691: how accessibility works and the general responsibility of these two
classes
- #1915: how we pulled these Accessibility Providers into a shared area
TL;DR: `ScreenInfoUiaProvider` lets you interact with the displayed text.
`UiaTextRange` is specific ranges of text in the display and navigate the text.
Thankfully, we didn't do many changes here. I feel like some of it is hacked
together but now that we have a somewhat working system, making changes
shouldn't be too hard...I hope.
#### UiaTextRange
We don't have access to the window handle. We really only need it to draw the
bounding rects using WinUser's `ScreenToClient()` and `ClientToScreen()`. I
need to figure out how to get around this.
In the meantime, I made the window handle optional. And if we don't have
one....well, we need to figure that out. But other than that, we have a
`UiaTextRange`.
#### ScreenInfoUiaProvider
At some point, we need to hook up this automation provider to the
WindowUiaProvider. This should help with navigation of the UIA Tree and make
everything just look waaaay better. For now, let's just do the same approach
and make the pUiaParent optional.
This one's the one I'm not that proud of, but it works. We need the parent to
get a bounding rect of the terminal. While we figure out how to attach the
WindowUiaProvider, we should at the very least be able to get a bunch of info
from our xaml automation peer. So, I've added a _getBoundingRect optional
function. This is what's called when we don't have a WindowUiaProvider as our
parent.
## Validation Steps Performed
I've been using inspect.exe to see the UIA tree.
I was able to interact with the terminal mostly fine. A few known issues below.
Unfortunately, I tried running Narrator on this and it didn't seem to like it
(by that I mean WT crashed). Then again, I don't really know how to use
narrator other than "click on object" --> "listen voice". I feel like there's a
way to get the other interactions with narrator, but I'll be looking into more
of that soon. I bet if I fix the two issues below, Narrator will be happy.
## Miscellaneous Known Issues
- `GetSelection()` and `GetVisibleRanges()` crashes. I need to debug through
these. I want to include them in this PR.
Fixes#1353.
