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

Console Terminal "Cursor Shape" gets reset when opening "Options" propsheet #1219

Closed
zadjii-msft opened this issue Jun 12, 2019 · 2 comments · Fixed by #2663
Closed

Console Terminal "Cursor Shape" gets reset when opening "Options" propsheet #1219

zadjii-msft opened this issue Jun 12, 2019 · 2 comments · Fixed by #2663
Assignees
Labels
Area-Settings Issues related to settings and customizability, for console or terminal Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-2 A description (P2) Product-Conhost For issues in the Console codebase Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Milestone

Comments

@zadjii-msft
Copy link
Member

Moved from MSFT:21001042

  1. go to properties / terminal

  2. select vertical bar

  3. click ok

at this point everything is good.

  1. go to properties / terminal

  2. go to option tab

  3. go to terminal tab

at this point you'll see that the cursor shape is reset to "legacy style". this only happens if you go to the "options tab". i think this happens because the options page as the "cursor size" option that is interfering -- just a guess.

@zadjii-msft zadjii-msft added Product-Conhost For issues in the Console codebase Issue-Bug It either shouldn't be doing this or needs an investigation. labels Jun 12, 2019
@zadjii-msft zadjii-msft added this to the 20H1 milestone Jun 12, 2019
@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 Jun 12, 2019
@zadjii-msft zadjii-msft added Area-Server Down in the muck of API call servicing, interprocess communication, eventing, etc. Area-Settings Issues related to settings and customizability, for console or terminal and removed Needs-Tag-Fix Doesn't match tag requirements Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Jun 12, 2019
@zadjii-msft zadjii-msft self-assigned this Aug 27, 2019
@zadjii-msft
Copy link
Member Author

So this is more painful than I thought it was going to be.

You'd think it'd be easy enough to just not select a radio button from the group of legacy cursor sizes when the cursor style is set to something other than "legacy". Unfortunately, I don't think ComCtl lets you have a group of radio buttons where one isn't selected.

When you open the "options" page, the options page gets a WM_COMMAND. This WM_COMMAND is a BN_CLICKED sent to the selected[1] radio button from that group[2]. Unfortunately, this BN_CLICKED is indistinguishable from a real click. So we can't really ignore that message. We also however can't handle that message, because our handling of a click on IDD_CURSOR_SMALL means setting the cursor style back to Legacy.

I tried ignoring the first BN_CLICKED sent to the cursor size radio buttons, but that wouldn't work if you switched tabs. There's probably a workaround I haven't thought of here. My comctl-fu is definitely not passable, so I might be able to research more and find something. Maybe I can add a fake 0 height radio button as part of the group, as a placeholder for "non-legacy"

Technically, what we're doing here is a real anti-pattern. We shouldn't have one group of radio buttons that are affected by another group like this. We especially shouldn't have them on different pages. If we wanted to re-arrange that dialog, we'd almost certainly need to wait until the next (post-20H1) release to do that, to account for localization changes.

[1]: presumably the last selected. It might also just be sent to the first button for the case where a group has no buttons selected.
[2]: Apparently this is documented behavior

@zadjii-msft zadjii-msft removed the Area-Server Down in the muck of API call servicing, interprocess communication, eventing, etc. label Sep 4, 2019
zadjii-msft added a commit that referenced this issue Sep 4, 2019
@ghost ghost added the In-PR This issue has a related PR label Sep 5, 2019
@DHowett-MSFT DHowett-MSFT added the Priority-2 A description (P2) label Sep 5, 2019
@ghost ghost closed this as completed in #2663 Sep 9, 2019
@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 Sep 9, 2019
ghost pushed a commit that referenced this issue Sep 9, 2019
* 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
@ExE-Boss
Copy link

ExE-Boss commented Sep 9, 2019

Now all we need is for PowerShell/PSReadLine#903 to be resolved.

DHowett-MSFT pushed a commit that referenced this issue Sep 23, 2019
* 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

(cherry picked from commit ce34c73)
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Settings Issues related to settings and customizability, for console or terminal Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-2 A description (P2) Product-Conhost For issues in the Console codebase Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants