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

Terminal crashes when navigating in empty palette #8421

Closed
Don-Vito opened this issue Nov 27, 2020 · 2 comments
Closed

Terminal crashes when navigating in empty palette #8421

Don-Vito opened this issue Nov 27, 2020 · 2 comments
Labels
Area-CmdPal Command Palette issues and features Area-User Interface Issues pertaining to the user interface of the Console or Terminal Issue-Bug It either shouldn't be doing this or needs an investigation. Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. Severity-Crash Crashes are real bad news.
Milestone

Comments

@Don-Vito
Copy link
Contributor

Don-Vito commented Nov 27, 2020

Environment

Windows Terminal version (if applicable): both 1.4 and 1.5

Steps to reproduce

  1. Open command palette
  2. Type filter that is not matched by any command
  3. Press "UP"
  4. Enjoy postmortem debugger

The reason is double bug here

void CommandPalette::SelectNextItem(const bool moveDown)
    {
        const auto selected = _filteredActionsView().SelectedIndex();
        const int numItems = ::base::saturated_cast<int>(_filteredActionsView().Items().Size());
       
        const auto newIndex = ((numItems + selected + (moveDown ? 1 : -1)) % numItems);
        _filteredActionsView().SelectedIndex(newIndex);
        _filteredActionsView().ScrollIntoView(_filteredActionsView().SelectedItem());
    }

We both do the math when numItems=0, and when we go up starting from selected=-1.

I have a fix here: #8420.
But I will probably extract it to a separate PR.

@ghost ghost added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels Nov 27, 2020
@ghost ghost added the In-PR This issue has a related PR label Nov 27, 2020
@zadjii-msft
Copy link
Member

G U H

Yep that's an easily repro-able crash alright, thanks for hoping on the fix so quick

@zadjii-msft zadjii-msft added Area-User Interface Issues pertaining to the user interface of the Console or Terminal Issue-Bug It either shouldn't be doing this or needs an investigation. Product-Terminal The new Windows Terminal. Severity-Crash Crashes are real bad news. labels Nov 29, 2020
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Nov 29, 2020
@zadjii-msft zadjii-msft added this to the Terminal v1.6 milestone Nov 29, 2020
@DHowett DHowett removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Nov 30, 2020
@zadjii-msft zadjii-msft added the Area-CmdPal Command Palette issues and features label Dec 1, 2020
@ghost ghost added Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. and removed In-PR This issue has a related PR labels Dec 1, 2020
@ghost ghost closed this as completed in 3a5042a Dec 1, 2020
@ghost
Copy link

ghost commented Jan 28, 2021

🎉This issue was addressed in #8422, which has now been successfully released as Windows Terminal Preview v1.6.10272.0.:tada:

Handy links:

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-CmdPal Command Palette issues and features Area-User Interface Issues pertaining to the user interface of the Console or Terminal Issue-Bug It either shouldn't be doing this or needs an investigation. Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. Severity-Crash Crashes are real bad news.
Projects
None yet
Development

No branches or pull requests

3 participants