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

Warn the user if the keyboard service is disabled #8095

Merged
10 commits merged into from
Nov 4, 2020

Conversation

zadjii-msft
Copy link
Member

@zadjii-msft zadjii-msft commented Oct 29, 2020

Summary of the Pull Request

kb-service-disabled

With this PR, the Terminal will check to make sure the "Touch, Keyboard and Handwriting Panel Service" is enabled at startup. If it isn't, then the Terminal won't be able to receive keyboard input (see #4448 and the 20 linked issues to that one).

References

PR Checklist

Validation Steps Performed

I manually set the service to "Disabled", restarted the machine, verified the dialog opens (and that I'm unable to type in the Terminal), then re-set the service to automatic and rebooted, and the dialog doesn't appear.

@ghost ghost added Area-User Interface Issues pertaining to the user interface of the Console or Terminal Issue-Task It's a feature request, but it doesn't really need a major design. Priority-3 A description (P3) Product-Terminal The new Windows Terminal. labels Oct 29, 2020
@carlos-zamora
Copy link
Member

We should probably add a section in the docs under troubleshooting for this. Should we provide a link to that in the popup dialog?

Also, I'm guessing the answer is "no" but do we want a "disable this warning" setting?

@DHowett
Copy link
Member

DHowett commented Oct 29, 2020

The way to disable this warning is to enable that service.

src/cascadia/TerminalApp/AppLogic.cpp Outdated Show resolved Hide resolved
src/cascadia/TerminalApp/AppLogic.cpp Outdated Show resolved Hide resolved
src/cascadia/TerminalApp/AppLogic.cpp Outdated Show resolved Hide resolved
src/cascadia/TerminalApp/Resources/en-US/Resources.resw Outdated Show resolved Hide resolved
@ghost ghost added Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something and removed Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something labels Oct 30, 2020
@zadjii-msft zadjii-msft self-assigned this Nov 2, 2020
@zadjii-msft
Copy link
Member Author

So as it turns out, the linter is set to auto pull the latest version, which is of course, broken. IMO, that's the last straw with the linter - I don't think we've gotten anything valuable from it frankly.

@zadjii-msft zadjii-msft assigned DHowett and miniksa and unassigned zadjii-msft Nov 3, 2020
Comment on lines +2682 to +2694
DWORD cchBuffer = 0;
GetServiceDisplayName(hManager.get(), TabletInputServiceKey.data(), nullptr, &cchBuffer);
std::wstring buffer;
cchBuffer += 1; // Add space for a null
buffer.resize(cchBuffer);

if (LOG_LAST_ERROR_IF(!GetServiceDisplayName(hManager.get(),
TabletInputServiceKey.data(),
buffer.data(),
&cchBuffer)))
{
return winrt::hstring{ TabletInputServiceKey };
}
Copy link
Member

@DHowett DHowett Nov 4, 2020

Choose a reason for hiding this comment

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

You might appreciate this helper!

std::wstring serviceName;
wil::AdaptFixedSizeToAllocatedResult(serviceName, [hManager](PWSTR buffer, size_t size, size_t* needed) -> HRESULT {
    DWORD cchBuffer{ static_cast<DWORD>(size) };
    BOOL const success = GetServiceDisplayName(hManager.get(), TabletInputServiceKey.data(), buffer, &cchBuffer);
    RETURN_LAST_ERROR_IF((success == FALSE) && (::GetLastError() != ERROR_INSUFFICIENT_BUFFER));
    *needed = cchBuffer = 1; // +1 for the null terminator
    return S_OK;
});

(this is a nit; I just think it's a cool helper!)

Copy link
Member

Choose a reason for hiding this comment

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

reckon you've gotta check the HR return from AdaptFixedSize...

Comment on lines +2682 to +2694
DWORD cchBuffer = 0;
GetServiceDisplayName(hManager.get(), TabletInputServiceKey.data(), nullptr, &cchBuffer);
std::wstring buffer;
cchBuffer += 1; // Add space for a null
buffer.resize(cchBuffer);

if (LOG_LAST_ERROR_IF(!GetServiceDisplayName(hManager.get(),
TabletInputServiceKey.data(),
buffer.data(),
&cchBuffer)))
{
return winrt::hstring{ TabletInputServiceKey };
}
Copy link
Member

Choose a reason for hiding this comment

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

reckon you've gotta check the HR return from AdaptFixedSize...

src/cascadia/TerminalApp/TerminalPage.cpp Outdated Show resolved Hide resolved
Co-authored-by: Dustin L. Howett <duhowett@microsoft.com>
@zadjii-msft zadjii-msft added the AutoMerge Marked for automatic merge by the bot when requirements are met label Nov 4, 2020
@ghost
Copy link

ghost commented Nov 4, 2020

Hello @zadjii-msft!

Because this pull request has the AutoMerge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

@ghost ghost merged commit d5d2b77 into main Nov 4, 2020
@ghost ghost deleted the dev/migrie/f/7886-warn-if-youve-turned-off-the-keyboard branch November 4, 2020 21:44
DHowett pushed a commit that referenced this pull request Nov 9, 2020
![kb-service-disabled](https://user-images.githubusercontent.com/18356694/97578533-eb792d80-19be-11eb-9b13-b771327a72a0.png)

With this PR, the Terminal will check to make sure the "Touch, Keyboard and Handwriting Panel Service" is enabled at startup. If it isn't, then the Terminal won't be able to receive keyboard input (see #4448 and the 20 linked issues to that one).

* See #4448 for more details

* [x] Closes #7886
* [ ] Should this make #4448 not-open as well?
* [x] I work here
* [n/a] Tests added/passed
* [x] Docs: MicrosoftDocs/terminal#168

I manually set the service to "Disabled", restarted the machine, verified the dialog opens (and that I'm unable to type in the Terminal), then re-set the service to automatic and rebooted, and the dialog doesn't appear.

(cherry picked from commit d5d2b77)
@ghost
Copy link

ghost commented Nov 11, 2020

🎉Windows Terminal v1.4.3141.0 has been released which incorporates this pull request.:tada:

Handy links:

@ghost
Copy link

ghost commented Nov 11, 2020

🎉Windows Terminal Preview v1.5.3142.0 has been released which incorporates this pull request.:tada:

Handy links:

DHowett pushed a commit that referenced this pull request Feb 22, 2021
Add a setting to turn off the warning if 'Touch Keyboard and
Handwriting Panel Service' is disabled.

The service might not start in some case, and it doesn't affect the
input in some computer.  This PR turn off the warning even if the
service is disabled.  The setting name is  "inputServiceWarning".

## Validation Steps Performed
I manually set the service to "Disabled", restarted the Terminal,
verified the warning up, then set "inputServiceWarning" to false and
restarted the Terminal, and the warning didn't appear.

References #8095
References #7886 (comment)
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-User Interface Issues pertaining to the user interface of the Console or Terminal AutoMerge Marked for automatic merge by the bot when requirements are met Issue-Task It's a feature request, but it doesn't really need a major design. Priority-3 A description (P3) Product-Terminal The new Windows Terminal.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add notice if service disabled: "Touch Keyboard and Handwriting Service"
5 participants