Skip to content

Commit

Permalink
Initialize cursorOn to match focus state (#12094)
Browse files Browse the repository at this point in the history
I'm not 100% sure that this is the right solution, but it does seem to work well enough. This is unfortunately a classic heisenbug. It was already hard enough to repro originally, but attaching a debugger made it totally impossible to hit.

My theory is that it's possible for the GotFocus event to fire before the LayoutUpdated event does. If that were to occur, then we'd try to turn on the cursor timer before it exists, gracefully do nothing, then create the timer. In that case, we'd never get a subsequent message to start the blinking.

I tested that theory by just initializing the cursor blinker to our `_focused` state. In that case, if the control has already been focused at the time of the LayoutUpdated event, then we can init the cursor to the correct state. Testing that out, I couldn't once get this to repro, which makes me think this works. I've opened some 900 (<sup>hyperbole</sup>) tabs now, so I'm pretty confident I'd have seen it by now.

* Regressed in #10978
* [x] fixes #11411
* [x] I made sure I didn't regress #6586
* [x] I work here

(cherry picked from commit 55aea08)
  • Loading branch information
zadjii-msft authored and DHowett committed Jan 21, 2022
1 parent d8170bc commit 6d86429
Showing 1 changed file with 10 additions and 2 deletions.
12 changes: 10 additions & 2 deletions src/cascadia/TerminalControl/TermControl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -692,8 +692,16 @@ namespace winrt::Microsoft::Terminal::Control::implementation
_cursorTimer.emplace(std::move(cursorTimer));
// As of GH#6586, don't start the cursor timer immediately, and
// don't show the cursor initially. We'll show the cursor and start
// the timer when the control is first focused. cursorTimer.Start();
_core.CursorOn(false);
// the timer when the control is first focused.
//
// As of GH#11411, turn on the cursor if we've already been marked
// as focused. We suspect that it's possible for the Focused event
// to fire before the LayoutUpdated. In that case, the
// _GotFocusHandler would mark us _focused, but find that a
// _cursorTimer doesn't exist, and it would never turn on the
// cursor. To mitigate, we'll initialize the cursor's 'on' state
// with `_focused` here.
_core.CursorOn(_focused);
}
else
{
Expand Down

0 comments on commit 6d86429

Please sign in to comment.