Skip to content

Merge master - #1

Merged
mcpiroman merged 156 commits into
mcpiroman:masterfrom
microsoft:master
Sep 16, 2019
Merged

Merge master#1
mcpiroman merged 156 commits into
mcpiroman:masterfrom
microsoft:master

Conversation

@mcpiroman

Copy link
Copy Markdown
Owner

No description provided.

carlos-zamora and others added 30 commits July 29, 2019 15:21
**The Basics of Accessibility**
- [What is a User Interaction Automation (UIA) Tree?](https://docs.microsoft.com/en-us/dotnet/framework/ui-automation/ui-automation-tree-overview)
- Other projects (i.e.: Narrator) can take advantage of this UIA tree and are used to present information within it.
- Some things like XAML already have a UIA Tree. So some UIA tree navigation and features are already there. It's just a matter of getting them hooked up and looking right.

**Accessibility in our Project**
There's a few important classes...
regarding Accessibility...
- **WindowUiaProvider**: This sets up the UIA tree for a window. So this is the top-level for the UIA tree.
- **ScreenInfoUiaProvider**: This sets up the UIA tree for a terminal buffer.
- **UiaTextRange**: This is essential to interacting with the UIA tree for the terminal buffer. Actually gets portions of the buffer and presents them.

regarding the Windows Terminal window...
- **BaseWindow**: The foundation to a window. Deals with HWNDs and that kind of stuff.
- **IslandWindow**: This extends `BaseWindow` and is actually what holds our Windows Terminal
- **NonClientIslandWindow**: An extension of the `IslandWindow`

regarding ConHost...
- **IConsoleWindow**: This is an interface for the console window.
- **Window**: This is the actual window for ConHost. Extends `IConsoleWindow`

- `IConsoleWindow` changes:
  - move into `Microsoft::Console::Types` (a shared space)
  - Have `IslandWindow` extend it
- `WindowUiaProvider` changes:
  - move into `Microsoft::Console::Types` (a shared space)
- Hook up `WindowUiaProvider` to IslandWindow (yay! we now have a tree)

### Changes to the WindowUiaProvider
As mentioned earlier, the WindowUiaProvider is the top-level UIA provider for our projects. To reuse as much code as possible, I created `Microsoft::Console::Types::WindowUiaProviderBase`. Any existing functions that reference a `ScreenInfoUiaProvider` were virtual-ized.

In each project, a `WindowUiaProvider : WindowUiaProviderBase` was created to define those virtual functions. Note that that will be the main difference between ConHost and Windows Terminal moving forward: how many TextBuffers are on the screen.

So, ConHost should be the same as before, with only one `ScreenInfoUiaProvider`, whereas Windows Terminal needs to (1) update which one is on the screen and (2) may have multiple on the screen.

🚨 Windows Terminal doesn't have the `ScreenInfoUiaProvider` hooked up yet. We'll have all the XAML elements in the UIA tree. But, since `TermControl` is a custom XAML Control, I need to hook up the `ScreenInfoUiaProvider` to it. This work will be done in a new PR and resolve GitHub Issue #1352.


### Moved to `Microsoft::Console::Types`
These files got moved to a shared area so that they can be used by both ConHost and Windows Terminal.
This means that any references to the `ServiceLocator` had to be removed.

- `IConsoleWindow`
  - Windows Terminal: `IslandWindow : IConsoleWindow`
- `ScreenInfoUiaProvider`
  - all references to `ServiceLocator` and `SCREEN_INFORMATION` were removed. `IRenderData` was used to accomplish this. Refer to next section for more details.
- `UiaTextRange`
  - all references to `ServiceLocator` and `SCREEN_INFORMATION` were removed. `IRenderData` was used to accomplish this. Refer to next section for more details.
  - since most of the functions were `static`, that means that an `IRenderData` had to be added into most of them.


### Changes to IRenderData
Since `IRenderData` is now being used to abstract out `ServiceLocator` and `SCREEN_INFORMATION`, I had to add a few functions here:
- `bool IsAreaSelected()`
- `void ClearSelection()`
- `void SelectNewRegion(...)`
- `HRESULT SearchForText(...)`

`SearchForText()` is a problem here. The overall new design is great! But Windows Terminal doesn't have a way to search for text in the buffer yet, whereas ConHost does. So I'm punting on this issue for now. It looks nasty, but just look at all the other pretty things here. :)
This commit adds some tracelogging (and telemetry) to answer the following questions:
* Do people use padding? If so, what is the common range of values?
* Are people turning off showTabsInTitlebar?
* How many different profiles are in use, and how do they break down between custom and default?
* Are people manually launching specific profiles, or using "default" fairly often?
* Are people using the Azure Cloud Shell connection?
* Are people leveraging the feature added in #2108 (autogenerating GUIDs)?
* Stop the crash with fonts by trying a few fallback/backup fonts if we can't find what was selected.
* Create fallback pattern for finding a font. Resolve and pass the locale name. Retrieve the font name while retrieving the font object. Use retrieved data in the _GetProposedFont methods instead of re-resolving it.
* Add details to schema about fallback. Finish comment explaining fallback pattern to doc comment on method.
* Don't trigger a frame due to circling when in the middle of a resize operation

  This fixes #1795, and shined quite a bit of light on the whole conpty resize process.

* Move the Begin/End to ResizeScreenBuffer, to catch more cases.
When we snap across a DPI boundary, we'll get the DPI changed message _after_ the resize message. So when we try to calculate the new terminal position, we'll use the _old_ DPI to calculate the size. When snapping to a lower DPI, this means the terminal will be smaller, with "padding" all around the actual app. 

Instead, when we get a new DPI, force us to update out UI layout for the new DPI.

Closes #2057
…and InsertCharacter (#2144)

* We now support EraseInLine, EraseInDisplay, DeleteCharacter and InsertCharacter
Acked-by: Pankaj Bhojwani <t-pabhoj@microsoft.com>
Acked-by: Carlos Zamora <cazamor@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.
* Attempt to remove all test and utility projects from audit mode (and turn it back on) to see if that keeps it within the disk space boundaries.
* drop x86 and arm configs for the test projects too.
* First draft of a spec for splitting off the existing VT52 escape sequences, and extending the VT52 support.

* Make the issue ID visible on GitHub.

* Added suggested mappings for the Graphics Mode character set.

* Add escape sequences for all the commands and clarify the use of the ESC < sequence when switching back to ANSI mode.

* Add details about the differing boundary rules of the VT100 CUP command and the VT52 Direct Cursor Address command.

* Specify the identifying sequence that the Identify command should return.

* Add details of the print commands.

* Add a list of keyboard sequences that are different in the VT52 mode, and make the description of the Keypad Mode commands a little clearer.

* Add a section describing the testing needed to cover the new functionality.
* Add information on WxH character

* Add line and separate footnote
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>
It's a doc change and the x86 CI is cranky. We're looking into it.
We added a few more rules. Update the bot doc description.
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.
* Try createing a script to only build the current working directory

  Inspired by #2078.

  I wanted to use this for WindowsTerminal, but I can't generate the
  resources.pri from just building WindowsTerminal. Maybe @DHowett-MSFT has
  some ideas.

* Cleanup for PR

* fix some bugs with building outside a project directory.

* PR nits
…space (#2271)

* 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.
)

* -  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
Microsoft.VCRTForwarders.140 1.0.0-rc -> 1.0.1-rc
Microsoft.Toolkit.Win32.UI.XamlApplication 6.0.0-preview6.* -> 6.0.0-preview7
Microsoft.Windows.CppWinRT 2.0.190605.7 -> 2.0.190730.2

wil fbcd1d2a -> e8c599bc
gsl b74b286d -> 1212beae

We're skipping the following update:
Microsoft.UI.Xaml 2.2.190611001-prerelease -> 2.2.190731001-prerelease
* 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
* Add list of keybindings to

* Add missed copy bindings
Move the hack from TerminalApp.vcxproj to a .targets file to be used by the ut_app project too.

Fixes #2143
kojoru and others added 29 commits September 4, 2019 11:45
This adds a few lines to support shorthand color hex codes like #ABC. They are treated as equivalent of #AABBCC.

Fixes #2639.
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.
* 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
…hey were a bit too challenging to solve with this giant PR
…verted-to-array type doesn't give the correct value.
## 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.
Adds a number of TL events we can use to track startup time better. Adds events for:
* Initial exe start
* Time the window is created
* time we start loading settings
* time we finish loading setings
* time when a connection recieves its first byte

Also updates our `ConnectionCreated` event to include the session GUID, so that we can correlate that with the connection's `RecievedFirstByte` event.
We were using a tag to trigger the bot for the verbose feedback hub response.

But...
1. We have run into several instances of the bot aggressively replying multiple times before the tag is removed.
2. We asked for a "comment contains" function in the bot and the Fabric Bot team obliged.

So I've changed it to `/duplicate` from the tag trigger and will remove the tag.
* Amends user-docs procedure

Amends docs procedure for `Running a Different Shell`:
* Adds an overview sentence.
* Adds some light rephrasing.
* Proposes using the countersink arrow `⌵` to depict the `down` GUI element.

* Adds link to WSL installation guide
* this actually fixes #1219

* the terminal page should check the checkbox on the options page

* Discard these changes from #2651

* Add comments, pull function out to helper
* This fixes #832 by not mucking with roundtripping attributes. Still needs a test

* Add a test

* Lets just make this test test everything

  @miniksa https://media0.giphy.com/media/d7mMzaGDYkz4ZBziP6/giphy.gif

* Remove dead code
Open Source program office guidelines encourage us to add this information to our repository. So I'm doing it.

Sourced from https://github.com/microsoft/microsoft.github.io/blob/master/SECURITY.MD
* Fix json settings documentation

The ctrl+c issue was fixed in [#2446](#2446)

* Update UsingJsonSettings.md
There are a number of VT escape sequences that rely on the `ScrollRegion`
function to scroll the viewport (RI, DL, IL, SU, SD, ICH, and DCH) , and all of
them have got the clipping rect or scroll boundaries wrong in some way,
resulting in content being scrolled off the screen that should have been
clipped, revealed areas not being correctly filled, or parts of the screen not
being moved that should have been. This PR attempts to fix all of those issues.

The `ScrollRegion` function is what ultimately handles the scrolling, but it's
typically called via the `ApiRoutines::ScrollConsoleScreenBufferWImpl` method,
and it's the callers of that method that have needed correcting.

One "mistake" that many of these operations made, was in setting a clipping
rect that was different from the scrolling rect. This should never have been
necessary, since the area being scrolled is also the boundary into which the
content needs to be clipped, so the easiest thing to do is just use the same
rect for both parameters.

Another common mistake was in clipping the horizontal boundaries to the width
of the viewport. But it's really the buffer width that represents the active
width of the screen - the viewport width and offset are merely a window on that
active area. As such, the viewport should only be used to clip vertically - the
horizontal extent should typically be the full buffer width.

On that note, there is really no need to actually calculate the buffer width
when we want to set any of the scrolling parameters to that width. The
`ScrollRegion` function already takes care of clipping everything within the
buffer boundary, so we can simply set the `Left` of the rect to `0` and the
`Right` to `SHORT_MAX`.

More details on individual commands:

* RI (the `DoSrvPrivateReverseLineFeed` function)
  This now uses a single rect for both the scroll region and clipping boundary,
  and the width is set to `SHORT_MAX` to cover the full buffer width. Also the
  bottom of the scrolling region is now the bottom of the viewport (rather than
  bottom-1), otherwise it would be off by one.

* DL and IL (the `DoSrvPrivateModifyLinesImpl` function)
  Again this uses a single rect for both the scroll region and clipping
  boundary, and the width is set to `SHORT_MAX` to cover the full width. The
  most significant change, though, is that the bottom boundary is now the
  viewport bottom rather than the buffer bottom. Using the buffer bottom
  prevented it clipping the content that scrolled off screen when inserting,
  and failed to fill the revealed area when deleting.

* SU and SD (the `AdaptDispatch::_ScrollMovement` method)
  This was already using a single rect for both the scroll region and clipping
  boundary, but it was previously constrained to the width of the viewport
  rather than the buffer width, so some areas of the screen weren't correctly
  scrolled. Also, the bottom boundary was off by 1, because it was using an
  exclusive rect while the `ScrollRegion` function expects inclusive rects.

* ICH and DCH (the `AdaptDispatch::_InsertDeleteHelper` method)
  This method has been considerably simplified, because it was reimplementing a
  lot of functionality that was already provided by the `ScrollRegion`
  function. And like many of the other cases, it has been updated to use a
  single rect for both the scroll region and clipping boundary, and clip to the
  full buffer width rather than the viewport width.

I should add that if we were following the specs exactly, then the SU and SD
commands should technically be panning the viewport over the buffer instead of
moving the buffer contents within the viewport boundary. So SU would be the
equivalent of a newline at the bottom of the viewport (assuming no margins).
And SD would assumedly do the opposite, scrolling the back buffer back into
view (an RI at the top of the viewport should do the same).

This doesn't seem to be something that is consistently implemented, though.
Some terminals do implement SU as a viewport pan, but I haven't seen anyone
implement SD or RI as a pan. If we do want to do something about this, I think
it's best addressed as a separate issue.

## Validation Steps Performed

There were already existing tests for the SU, SD, ICH, and DCH commands, but
they were implemented as adapter tests, which weren't effectively testing
anything - the `ScrollConsoleScreenBufferW` method used in those tests was just
a mock (an incomplete reimplementation of the `ScrollRegion` function), so
confirming that the mock produced the correct result told you nothing about the
validity of the real code.

To address that, I've now reimplemented those adapter tests as screen buffer
tests. For the most part I've tried to duplicate the functionality of the
original tests, but there are significant differences to account for the fact
that scrolling region now covers the full width of the buffer rather than just
the viewport width.

I've also extended those tests with additional coverage for the RI, DL, and IL
commands, which are really just a variation of the SU and SD functionality.

Closes #2174
* Edits doc section `Configuring Windows Terminal`

* Converts into a procedure.
* Uses `⌵` character to replace the `down` UI element.

* Additional minor edit

Updates formatting, edits for brevity.

* Fixed json path

Added `8wekyb3d8bbwe` to file path.
* Move cursor position to the left margin after execution of the IL and DL escape sequences.
* Update IL and DL screen buffer tests to account for the cursor moving to the left margin.
CLS calls two functions: 
- `SetConsoleCursorPositionImpl()`
- `ScrollConsoleScreenBufferWImpl()`
Both of these were not checking which buffer to apply to (main vs active buffer).

Now we get the active buffer and apply the changes to that one.

Also, we forgot to switch out of the alt buffer in the previous test. Added that in.

Closes #1189.
@mcpiroman
mcpiroman merged commit ba4561c into mcpiroman:master Sep 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.