Skip to content

Commit

Permalink
Abstract GetTextForClipboard() for UIA (#4578)
Browse files Browse the repository at this point in the history
## Summary of the Pull Request
`GetTextForClipboard` already exists in the TextBuffer. It makes sense to use that for UIA as well. This changes the behavior or `GetText()` such that it does not remove leading/trailing whitespace anymore. That is more of an expected behavior.

## References
This also contributes to...
- #4509: UIA Box Selection
- #2447: UIA Signaling for Selection
- #1354: UIA support for Wide Glyphs
Now that the expansion occurs at before render-time, the selection anchors are an accurate representation of what is selected. We just need to move GetText to the TextBuffer. Then we can have those three issues just rely on code from the text buffer. This also means ConHost gets some of this stuff for free 😀

## Detailed Description of the Pull Request / Additional comments
- `TextBuffer::GetTextForClipboard()` --> `GetText()`
- `TextBuffer::GetText()` no longer requires GetForegroundColor/GetBackgroundColor. If either of these are not defined, we return a `TextAndColor` with only the `text` field populated.
- renamed a few parameters for copying text to the clipboard for clarity
- Updated `UiaTextRange::GetText()` to use `TextBuffer::GetText()`

## Validation Steps Performed
Manual tests for UIA using accessibility insights and Windows Terminal's copy action (w/ and w/out shift)

Added tests as well.
  • Loading branch information
carlos-zamora authored Mar 9, 2020
1 parent 4834604 commit e79a421
Show file tree
Hide file tree
Showing 13 changed files with 353 additions and 166 deletions.
100 changes: 60 additions & 40 deletions src/buffer/out/textBuffer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1364,26 +1364,30 @@ void TextBuffer::_ExpandTextRow(SMALL_RECT& textRow) const
// Routine Description:
// - Retrieves the text data from the selected region and presents it in a clipboard-ready format (given little post-processing).
// Arguments:
// - lineSelection - true if entire line is being selected. False otherwise (box selection)
// - trimTrailingWhitespace - setting flag removes trailing whitespace at the end of each row in selection
// - selectionRects - the selection regions from which the data will be extracted from the buffer
// - GetForegroundColor - function used to map TextAttribute to RGB COLORREF for foreground color
// - GetBackgroundColor - function used to map TextAttribute to RGB COLORREF for foreground color
// - includeCRLF - inject CRLF pairs to the end of each line
// - trimTrailingWhitespace - remove the trailing whitespace at the end of each line
// - textRects - the rectangular regions from which the data will be extracted from the buffer (i.e.: selection rects)
// - GetForegroundColor - function used to map TextAttribute to RGB COLORREF for foreground color. If null, only extract the text.
// - GetBackgroundColor - function used to map TextAttribute to RGB COLORREF for background color. If null, only extract the text.
// Return Value:
// - The text, background color, and foreground color data of the selected region of the text buffer.
const TextBuffer::TextAndColor TextBuffer::GetTextForClipboard(const bool lineSelection,
const bool trimTrailingWhitespace,
const std::vector<SMALL_RECT>& selectionRects,
std::function<COLORREF(TextAttribute&)> GetForegroundColor,
std::function<COLORREF(TextAttribute&)> GetBackgroundColor) const
const TextBuffer::TextAndColor TextBuffer::GetText(const bool includeCRLF,
const bool trimTrailingWhitespace,
const std::vector<SMALL_RECT>& selectionRects,
std::function<COLORREF(TextAttribute&)> GetForegroundColor,
std::function<COLORREF(TextAttribute&)> GetBackgroundColor) const
{
TextAndColor data;
const bool copyTextColor = GetForegroundColor && GetBackgroundColor;

// preallocate our vectors to reduce reallocs
size_t const rows = selectionRects.size();
data.text.reserve(rows);
data.FgAttr.reserve(rows);
data.BkAttr.reserve(rows);
if (copyTextColor)
{
data.FgAttr.reserve(rows);
data.BkAttr.reserve(rows);
}

// for each row in the selection
for (UINT i = 0; i < rows; i++)
Expand All @@ -1402,60 +1406,73 @@ const TextBuffer::TextAndColor TextBuffer::GetTextForClipboard(const bool lineSe

// preallocate to avoid reallocs
selectionText.reserve(gsl::narrow<size_t>(highlight.Width()) + 2); // + 2 for \r\n if we munged it
selectionFgAttr.reserve(gsl::narrow<size_t>(highlight.Width()) + 2);
selectionBkAttr.reserve(gsl::narrow<size_t>(highlight.Width()) + 2);
if (copyTextColor)
{
selectionFgAttr.reserve(gsl::narrow<size_t>(highlight.Width()) + 2);
selectionBkAttr.reserve(gsl::narrow<size_t>(highlight.Width()) + 2);
}

// copy char data into the string buffer, skipping trailing bytes
while (it)
{
const auto& cell = *it;
auto cellData = cell.TextAttr();
COLORREF const CellFgAttr = GetForegroundColor(cellData);
COLORREF const CellBkAttr = GetBackgroundColor(cellData);

if (!cell.DbcsAttr().IsTrailing())
{
selectionText.append(cell.Chars());
for (const wchar_t wch : cell.Chars())

if (copyTextColor)
{
selectionFgAttr.push_back(CellFgAttr);
selectionBkAttr.push_back(CellBkAttr);
auto cellData = cell.TextAttr();
COLORREF const CellFgAttr = GetForegroundColor(cellData);
COLORREF const CellBkAttr = GetBackgroundColor(cellData);
for (const wchar_t wch : cell.Chars())
{
selectionFgAttr.push_back(CellFgAttr);
selectionBkAttr.push_back(CellBkAttr);
}
}
}
#pragma warning(suppress : 26444)
// TODO GH 2675: figure out why there's custom construction/destruction happening here
it++;
}

// trim trailing spaces if SHIFT key not held
const bool forcedWrap = GetRowByOffset(iRow).GetCharRow().WasWrapForced();

if (trimTrailingWhitespace)
{
const ROW& Row = GetRowByOffset(iRow);

// FOR LINE SELECTION ONLY: if the row was wrapped, don't remove the spaces at the end.
if (!lineSelection || !Row.GetCharRow().WasWrapForced())
// if the row was NOT wrapped...
if (!forcedWrap)
{
// remove the spaces at the end (aka trim the trailing whitespace)
while (!selectionText.empty() && selectionText.back() == UNICODE_SPACE)
{
selectionText.pop_back();
selectionFgAttr.pop_back();
selectionBkAttr.pop_back();
if (copyTextColor)
{
selectionFgAttr.pop_back();
selectionBkAttr.pop_back();
}
}
}
}

// apply CR/LF to the end of the final string, unless we're the last line.
// a.k.a if we're earlier than the bottom, then apply CR/LF.
if (i < selectionRects.size() - 1)
// apply CR/LF to the end of the final string, unless we're the last line.
// a.k.a if we're earlier than the bottom, then apply CR/LF.
if (includeCRLF && i < selectionRects.size() - 1)
{
// if the row was NOT wrapped...
if (!forcedWrap)
{
// FOR LINE SELECTION ONLY: if the row was wrapped, do not apply CR/LF.
// a.k.a. if the row was NOT wrapped, then we can assume a CR/LF is proper
// always apply \r\n for box selection
if (!lineSelection || !GetRowByOffset(iRow).GetCharRow().WasWrapForced())
{
COLORREF const Blackness = RGB(0x00, 0x00, 0x00); // cant see CR/LF so just use black FG & BK
// then we can assume a CR/LF is proper
selectionText.push_back(UNICODE_CARRIAGERETURN);
selectionText.push_back(UNICODE_LINEFEED);

selectionText.push_back(UNICODE_CARRIAGERETURN);
selectionText.push_back(UNICODE_LINEFEED);
if (copyTextColor)
{
// cant see CR/LF so just use black FG & BK
COLORREF const Blackness = RGB(0x00, 0x00, 0x00);
selectionFgAttr.push_back(Blackness);
selectionFgAttr.push_back(Blackness);
selectionBkAttr.push_back(Blackness);
Expand All @@ -1465,8 +1482,11 @@ const TextBuffer::TextAndColor TextBuffer::GetTextForClipboard(const bool lineSe
}

data.text.emplace_back(std::move(selectionText));
data.FgAttr.emplace_back(std::move(selectionFgAttr));
data.BkAttr.emplace_back(std::move(selectionBkAttr));
if (copyTextColor)
{
data.FgAttr.emplace_back(std::move(selectionFgAttr));
data.BkAttr.emplace_back(std::move(selectionBkAttr));
}
}

return data;
Expand Down
10 changes: 5 additions & 5 deletions src/buffer/out/textBuffer.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -145,11 +145,11 @@ class TextBuffer final
std::vector<std::vector<COLORREF>> BkAttr;
};

const TextAndColor GetTextForClipboard(const bool lineSelection,
const bool trimTrailingWhitespace,
const std::vector<SMALL_RECT>& selectionRects,
std::function<COLORREF(TextAttribute&)> GetForegroundColor,
std::function<COLORREF(TextAttribute&)> GetBackgroundColor) const;
const TextAndColor GetText(const bool lineSelection,
const bool trimTrailingWhitespace,
const std::vector<SMALL_RECT>& textRects,
std::function<COLORREF(TextAttribute&)> GetForegroundColor = nullptr,
std::function<COLORREF(TextAttribute&)> GetBackgroundColor = nullptr) const;

static std::string GenHTML(const TextAndColor& rows,
const int fontHeightPoints,
Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalApp/TerminalPage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1345,7 +1345,7 @@ namespace winrt::TerminalApp::implementation
bool TerminalPage::_CopyText(const bool trimTrailingWhitespace)
{
const auto control = _GetActiveControl();
return control.CopySelectionToClipboard(trimTrailingWhitespace);
return control.CopySelectionToClipboard(!trimTrailingWhitespace);
}

// Method Description:
Expand Down
11 changes: 5 additions & 6 deletions src/cascadia/TerminalControl/TermControl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -903,7 +903,7 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
}
else
{
CopySelectionToClipboard(!shiftEnabled);
CopySelectionToClipboard(shiftEnabled);
}
}
}
Expand Down Expand Up @@ -1028,7 +1028,7 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
// If the terminal was unfocused AND a click-drag selection happened, copy to clipboard.
if (!_unfocusedClickPos || (_unfocusedClickPos && _isClickDragSelection))
{
CopySelectionToClipboard(!shiftEnabled);
CopySelectionToClipboard(shiftEnabled);
}
}
}
Expand Down Expand Up @@ -1661,17 +1661,16 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
// Windows Clipboard (CascadiaWin32:main.cpp).
// - CopyOnSelect does NOT clear the selection
// Arguments:
// - trimTrailingWhitespace: enable removing any whitespace from copied selection
// and get text to appear on separate lines.
bool TermControl::CopySelectionToClipboard(bool trimTrailingWhitespace)
// - collapseText: collapse all of the text to one line
bool TermControl::CopySelectionToClipboard(bool collapseText)
{
// no selection --> nothing to copy
if (_terminal == nullptr || !_terminal->IsSelectionActive())
{
return false;
}
// extract text from buffer
const auto bufferData = _terminal->RetrieveSelectedTextFromBuffer(trimTrailingWhitespace);
const auto bufferData = _terminal->RetrieveSelectedTextFromBuffer(collapseText);

// convert text: vector<string> --> string
std::wstring textData;
Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalControl/TermControl.h
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
hstring Title();
hstring GetProfileName() const;

bool CopySelectionToClipboard(bool trimTrailingWhitespace);
bool CopySelectionToClipboard(bool collapseText);
void PasteTextFromClipboard();
void Close();
Windows::Foundation::Size CharacterDimensions() const;
Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalControl/TermControl.idl
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ namespace Microsoft.Terminal.TerminalControl

String Title { get; };

Boolean CopySelectionToClipboard(Boolean trimTrailingWhitespace);
Boolean CopySelectionToClipboard(Boolean collapseText);
void PasteTextFromClipboard();
void Close();
Windows.Foundation.Size CharacterDimensions { get; };
Expand Down
27 changes: 14 additions & 13 deletions src/cascadia/TerminalCore/TerminalSelection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,15 @@ using namespace Microsoft::Terminal::Core;
* |-position where we double-clicked
* _|_
* |word|
* |--|
* |--|
* start & pivot-| |-end
*
* 2. Drag your mouse down a line
*
*
* start & pivot-|__________
*
* start & pivot-|__________
* __|word_______|
* |______|
* |______|
* |
* |-end & mouse position
*
Expand All @@ -33,7 +33,7 @@ using namespace Microsoft::Terminal::Core;
* |-start & mouse position
* |________
* ____| ______|
* |___w|ord
* |___w|ord
* |-end & pivot
*
* The pivot never moves until a new selection is created. It ensures that that cell will always be selected.
Expand Down Expand Up @@ -250,20 +250,21 @@ void Terminal::ClearSelection()
// Method Description:
// - get wstring text from highlighted portion of text buffer
// Arguments:
// - trimTrailingWhitespace: enable removing any whitespace from copied selection
// and get text to appear on separate lines.
// - collapseText: collapse all of the text to one line
// Return Value:
// - wstring text from buffer. If extended to multiple lines, each line is separated by \r\n
const TextBuffer::TextAndColor Terminal::RetrieveSelectedTextFromBuffer(bool trimTrailingWhitespace) const
const TextBuffer::TextAndColor Terminal::RetrieveSelectedTextFromBuffer(bool collapseText) const
{
const auto selectionRects = _GetSelectionRects();

std::function<COLORREF(TextAttribute&)> GetForegroundColor = std::bind(&Terminal::GetForegroundColor, this, std::placeholders::_1);
std::function<COLORREF(TextAttribute&)> GetBackgroundColor = std::bind(&Terminal::GetBackgroundColor, this, std::placeholders::_1);

return _buffer->GetTextForClipboard(!_blockSelection,
trimTrailingWhitespace,
_GetSelectionRects(),
GetForegroundColor,
GetBackgroundColor);
return _buffer->GetText(!collapseText,
!collapseText,
selectionRects,
GetForegroundColor,
GetBackgroundColor);
}

// Method Description:
Expand Down
6 changes: 2 additions & 4 deletions src/host/ut_host/ClipboardTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -84,10 +84,8 @@ class ClipboardTests
selection.emplace_back(SMALL_RECT{ 0, 2, 14, 2 });
selection.emplace_back(SMALL_RECT{ 0, 3, 8, 3 });

return Clipboard::Instance().RetrieveTextFromBuffer(screenInfo,
fLineSelection,
selection)
.text;
const auto& buffer = screenInfo.GetTextBuffer();
return buffer.GetText(true, fLineSelection, selection).text;
}

#pragma prefast(push)
Expand Down
Loading

0 comments on commit e79a421

Please sign in to comment.