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

Expose Text Attributes to UI Automation #10336

Merged
10 commits merged into from
Jul 9, 2021
5 changes: 5 additions & 0 deletions src/buffer/out/textBufferCellIterator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -265,3 +265,8 @@ const OutputCellView* TextBufferCellIterator::operator->() const noexcept
{
return &_view;
}

COORD TextBufferCellIterator::Pos() const noexcept
{
return _pos;
}
2 changes: 2 additions & 0 deletions src/buffer/out/textBufferCellIterator.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ class TextBufferCellIterator
const OutputCellView& operator*() const noexcept;
const OutputCellView* operator->() const noexcept;

COORD Pos() const noexcept;

carlos-zamora marked this conversation as resolved.
Show resolved Hide resolved
protected:
void _SetPos(const COORD newPos);
void _GenerateView();
Expand Down
2 changes: 2 additions & 0 deletions src/cascadia/TerminalControl/ControlCore.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -592,6 +592,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation
const int newDpi = static_cast<int>(static_cast<double>(USER_DEFAULT_SCREEN_DPI) *
_compositionScale);

_terminal->SetFontInfo(_actualFont);
carlos-zamora marked this conversation as resolved.
Show resolved Hide resolved

// TODO: MSFT:20895307 If the font doesn't exist, this doesn't
// actually fail. We need a way to gracefully fallback.
_renderer->TriggerFontChange(newDpi, _desiredFont, _actualFont);
Expand Down
50 changes: 46 additions & 4 deletions src/cascadia/TerminalControl/XamlUiaTextRange.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include "XamlUiaTextRange.h"
#include "../types/TermControlUiaTextRange.hpp"
#include <UIAutomationClient.h>
#include <UIAutomationCoreApi.h>

// the same as COR_E_NOTSUPPORTED
// we don't want to import the CLR headers to get it
Expand Down Expand Up @@ -89,12 +90,52 @@ namespace winrt::Microsoft::Terminal::Control::implementation

winrt::Windows::Foundation::IInspectable XamlUiaTextRange::GetAttributeValue(int32_t textAttributeId) const
{
// Copied functionality from Types::UiaTextRange.cpp
if (textAttributeId == UIA_IsReadOnlyAttributeId)
// Call the function off of the underlying UiaTextRange.
VARIANT result;
THROW_IF_FAILED(_uiaProvider->GetAttributeValue(textAttributeId, &result));

// Convert the resulting VARIANT into a format that is consumable by XAML.
switch (result.vt)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm surprised that nobody has made a C++ visitor specialization for a variant

{
case VT_BSTR:
{
return box_value(result.bstrVal);
}
case VT_I4:
{
// Surprisingly, `long` is _not_ a WinRT type.
// So we have to use `int32_t` to make sure this is output properly.
// Otherwise, you'll get "Attribute does not exist" out the other end.
return box_value<int32_t>(result.lVal);
}
case VT_R8:
{
return box_value(result.dblVal);
}
case VT_BOOL:
{
return winrt::box_value(false);
return box_value<bool>(result.boolVal);
}
else
case VT_UNKNOWN:
{
// This one is particularly special.
// We might return a special value like UiaGetReservedMixedAttributeValue
// or UiaGetReservedNotSupportedValue.
// Some text attributes may return a real value, however, none of those
// are supported at this time.
// So we need to figure out what was actually intended to be returned.

com_ptr<IUnknown> mixedAttributeVal;
UiaGetReservedMixedAttributeValue(mixedAttributeVal.put());

if (result.punkVal == mixedAttributeVal.get())
{
return Windows::UI::Xaml::DependencyProperty::UnsetValue();
}

[[fallthrough]];
}
default:
{
// We _need_ to return XAML_E_NOT_SUPPORTED here.
// Returning nullptr is an improper implementation of it being unsupported.
Expand All @@ -103,6 +144,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
// Magically, this doesn't affect other forms of navigation...
winrt::throw_hresult(XAML_E_NOT_SUPPORTED);
}
}
}

void XamlUiaTextRange::GetBoundingRectangles(com_array<double>& returnValue) const
Expand Down
1 change: 0 additions & 1 deletion src/cascadia/TerminalCore/Terminal.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
#include "../../terminal/parser/OutputStateMachineEngine.hpp"
#include "TerminalDispatch.hpp"
#include "../../inc/unicode.hpp"
#include "../../inc/DefaultSettings.h"
#include "../../inc/argb.h"
#include "../../types/inc/utils.hpp"
#include "../../types/inc/colorTable.hpp"
Expand Down
8 changes: 7 additions & 1 deletion src/cascadia/TerminalCore/Terminal.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

#include <conattrs.hpp>

#include "../../inc/DefaultSettings.h"
#include "../../buffer/out/textBuffer.hpp"
#include "../../types/inc/sgrStack.hpp"
#include "../../renderer/inc/BlinkingState.hpp"
Expand Down Expand Up @@ -68,6 +69,7 @@ class Microsoft::Terminal::Core::Terminal final :

void UpdateSettings(winrt::Microsoft::Terminal::Core::ICoreSettings settings);
void UpdateAppearance(const winrt::Microsoft::Terminal::Core::ICoreAppearance& appearance);
void SetFontInfo(const FontInfo& fontInfo);

// Write goes through the parser
void Write(std::wstring_view stringView);
Expand Down Expand Up @@ -160,6 +162,7 @@ class Microsoft::Terminal::Core::Terminal final :
COORD GetTextBufferEndPosition() const noexcept override;
const TextBuffer& GetTextBuffer() noexcept override;
const FontInfo& GetFontInfo() noexcept override;
std::pair<COLORREF, COLORREF> GetAttributeColors(const TextAttribute& attr) const noexcept override;

void LockConsole() noexcept override;
void UnlockConsole() noexcept override;
Expand All @@ -168,7 +171,6 @@ class Microsoft::Terminal::Core::Terminal final :
#pragma region IRenderData
// These methods are defined in TerminalRenderData.cpp
const TextAttribute GetDefaultBrushColors() noexcept override;
std::pair<COLORREF, COLORREF> GetAttributeColors(const TextAttribute& attr) const noexcept override;
COORD GetCursorPosition() const noexcept override;
bool IsCursorVisible() const noexcept override;
bool IsCursorOn() const noexcept override;
Expand Down Expand Up @@ -276,6 +278,10 @@ class Microsoft::Terminal::Core::Terminal final :
size_t _hyperlinkPatternId;

std::wstring _workingDirectory;

// This font value is only used to check if the font is a raster font.
// Otherwise, the font is changed with the renderer via TriggerFontChange.
carlos-zamora marked this conversation as resolved.
Show resolved Hide resolved
FontInfo _fontInfo{ DEFAULT_FONT_FACE, TMPF_TRUETYPE, 10, { 0, DEFAULT_FONT_SIZE }, CP_UTF8, false };
#pragma region Text Selection
// a selection is represented as a range between two COORDs (start and end)
// the pivot is the COORD that remains selected when you extend a selection in any direction
Expand Down
20 changes: 6 additions & 14 deletions src/cascadia/TerminalCore/terminalrenderdata.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,23 +27,15 @@ const TextBuffer& Terminal::GetTextBuffer() noexcept
return *_buffer;
}

// Creating a FontInfo can technically throw (on string allocation) and this is noexcept.
// That means this will std::terminate. We could come back and make there be a default constructor
// backup to FontInfo that throws no exceptions and allocates a default FontInfo structure.
#pragma warning(push)
#pragma warning(disable : 26447)
const FontInfo& Terminal::GetFontInfo() noexcept
{
// TODO: This font value is only used to check if the font is a raster font.
// Otherwise, the font is changed with the renderer via TriggerFontChange.
// The renderer never uses any of the other members from the value returned
// by this method.
// We could very likely replace this with just an IsRasterFont method
// (which would return false)
static const FontInfo _fakeFontInfo(DEFAULT_FONT_FACE, TMPF_TRUETYPE, 10, { 0, DEFAULT_FONT_SIZE }, CP_UTF8, false);
return _fakeFontInfo;
return _fontInfo;
}

void Terminal::SetFontInfo(const FontInfo& fontInfo)
{
_fontInfo = fontInfo;
}
Comment on lines +35 to 38
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wait hold up wat.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We specifically need this to get the font name (already in this PR) and the font size (excluded from this PR).

What's annoying is that this is a Terminal-specific issue too. ConHost returns an actual thing in GetFontInfo. Terminal, for whatever reason (we probably just didn't need it at the time tbh), lies.

We'd need some sort of dummy implementation layer that mostly just calls into Terminal for the majority of the implementation, and ControlCore just for that one method... that seems dumb.

What if we double down on this info being stored in TerminalCore? And anytime ControlCore needs something from it, it has to call _terminal.GetFontInfo()? That way this is all in one place. (this would have to be a follow-up)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we probably just didn't need it at the time tbh

The need for it was solely to get the renderer to "do the right thing". The terminal actually didn't need to know anything about its font at any point other than rendering.

All it did was jog the renderer into knowing that it was, in fact, not rendering a raster font.

Why we cared about that, I will never know.

#pragma warning(pop)

const TextAttribute Terminal::GetDefaultBrushColors() noexcept
{
Expand Down
3 changes: 1 addition & 2 deletions src/host/renderData.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ class RenderData final :
COORD GetTextBufferEndPosition() const noexcept override;
const TextBuffer& GetTextBuffer() noexcept override;
const FontInfo& GetFontInfo() noexcept override;
std::pair<COLORREF, COLORREF> GetAttributeColors(const TextAttribute& attr) const noexcept override;

std::vector<Microsoft::Console::Types::Viewport> GetSelectionRects() noexcept override;

Expand All @@ -37,8 +38,6 @@ class RenderData final :
#pragma region IRenderData
const TextAttribute GetDefaultBrushColors() noexcept override;

std::pair<COLORREF, COLORREF> GetAttributeColors(const TextAttribute& attr) const noexcept override;

COORD GetCursorPosition() const noexcept override;
bool IsCursorVisible() const noexcept override;
bool IsCursorOn() const noexcept override;
Expand Down
Loading