mirror of
https://github.com/microsoft/PowerToys.git
synced 2026-04-03 09:46:54 +02:00
[ZoomIt] Fix ampersand typing bug and debug assertion failure (#43679)
## Summary of the Pull Request This PR fixes two typing-related issues in ZoomIt: 1. Ampersands could be typed even when Type Mode or Draw Mode were not engaged 2. On Debug builds, typing a non-alphanumeric character in Type Mode would crash ZoomIt with a CRT assertion failure <!-- Please review the items on the PR checklist before submitting--> ## PR Checklist - [x] Closes: #43676 - [x] Closes: #43677 - [ ] **Communication:** I've discussed this with core contributors already. If the work hasn't been agreed, this work might be rejected - [ ] **Tests:** Added/updated and all pass - [ ] **Localization:** All end-user-facing strings can be localized - [ ] **Dev docs:** Added/updated - [ ] **New binaries:** Added on the required places - [ ] [JSON for signing](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ESRPSigning_core.json) for new binaries - [ ] [WXS for installer](https://github.com/microsoft/PowerToys/blob/main/installer/PowerToysSetup/Product.wxs) for new binaries and localization folder - [ ] [YML for CI pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ci/templates/build-powertoys-steps.yml) for new test projects - [ ] [YML for signed pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/release.yml) - [ ] **Documentation updated:** If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/windows-uwp/tree/docs/hub/powertoys) and link it here: #xxx ## Detailed Description of the Pull Request / Additional comments ### Assertion failure on Debug builds **Root Cause** This occurred because of a combination of a type-coercion issue and the use of `isprint()` in `WM_KEYDOWN`, which operates on virtual key codes, not characters. This is the code with the fault: ```cpp if( (g_TypeMode != TypeModeOff) && g_HaveTyped && static_cast<char>(wParam) != VK_UP && static_cast<char>(wParam) != VK_DOWN && (isprint( static_cast<char>(wParam)) || wParam == VK_RETURN || wParam == VK_DELETE || wParam == VK_BACK )) { ``` There are a few issues here: 1. There is no need for the `VK_UP` / `VK_DOWN` check. The block only executes if `VK_RETURN`, `VK_DELETE` or `VK_BACK` are pressed, which cannot be `VK_UP` or `VK_DOWN` by definition. This should be removed. 2. Casting the `wParam` to `char` means casting an unsigned int value to a signed char. This works for alphanumeric characters, as the VK_-codes correspond to their char counterparts. But it fails for values with their high bit set, e.g. a hyphen: - The virtual key code for the hyphen key is `VK_OEM_MINUS`, or `0xBD` - `0xBD` (10111101) becomes `-67` when cast to a char - In Debug builds, a call to `isprint()` includes a range check to ensure the value is in the range 1 to 255. A negative value trips this assertion. 3. The casts are not needed. **Fix** Remove both the `isprint()` call (the WM_CHAR handler has an `iswprint()` check) and remove the check against `VK_UP` and `VK_DOWN`. ### Ampersand issue This is a simple operator precedence issue with this statement: ```cpp if ((g_TypeMode != TypeModeOff) && iswprint(static_cast<TCHAR>(wParam)) || (static_cast<TCHAR>(wParam) == L'&')) ``` The intention is to continue if one of the Type Modes is engaged (either left-to-right or right-to-left) and either the typed character is printable or (a special-case) the ampersand (presumably for legacy issues when `DT_NOPREFIX` was not present on all draw text calls). Unfortunately, the parentheses are placed incorrectly, resulting in the expression actually being: `if (Type Mode is active AND a printable character was pressed) OR (Ampersand was pressed)` (Meaning the code will always execute if ampersand is pressed regardless of the mode.) **Fix** Correcting the placement of the parentheses fixes the issue. Note: I think `DT_NOPREFIX` exists on all `DrawText()` calls which render characters, so we could potentially remove the ampersand check entirely in the future, assuming that was the original issue which required the special casing. ## Validation Steps Performed - Ensure ampersand does not result in the character appearing and/or glitches occurring where the cursor is when Type Mode or Draw Mode are not active. - Ensure ampersands may still be typed as normal in Type Mode. - Confirm that non-alphanumeric characters can be typed without issue in Type Mode on both Debug and Release builds. - Test draw operations in combination with text notes. - Test backspace, return and delete keys in Type Mode. - Test that Type Mode engages repeatedly and can be exited.
This commit is contained in:
@@ -7296,7 +7296,8 @@ LRESULT APIENTRY MainWndProc(
|
||||
case WM_IME_CHAR:
|
||||
case WM_CHAR:
|
||||
|
||||
if( (g_TypeMode != TypeModeOff) && iswprint(static_cast<TCHAR>(wParam)) || (static_cast<TCHAR>(wParam) == L'&')) {
|
||||
if( (g_TypeMode != TypeModeOff) &&
|
||||
(iswprint(static_cast<TCHAR>(wParam)) || (static_cast<TCHAR>(wParam) == L'&')) ) {
|
||||
g_HaveTyped = TRUE;
|
||||
|
||||
TCHAR vKey = static_cast<TCHAR>(wParam);
|
||||
@@ -7404,9 +7405,8 @@ LRESULT APIENTRY MainWndProc(
|
||||
|
||||
case WM_KEYDOWN:
|
||||
|
||||
if( (g_TypeMode != TypeModeOff) && g_HaveTyped && static_cast<char>(wParam) != VK_UP && static_cast<char>(wParam) != VK_DOWN &&
|
||||
(isprint( static_cast<char>(wParam)) ||
|
||||
wParam == VK_RETURN || wParam == VK_DELETE || wParam == VK_BACK )) {
|
||||
if( (g_TypeMode != TypeModeOff) && g_HaveTyped &&
|
||||
(wParam == VK_RETURN || wParam == VK_DELETE || wParam == VK_BACK) ) {
|
||||
|
||||
if( wParam == VK_RETURN ) {
|
||||
|
||||
|
||||
Reference in New Issue
Block a user