[ZoomIt] Fix ampersand typing bug and debug assertion failure

[daverayment]

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

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 for new binaries
  • [ ] WXS for installer for new binaries and localization folder
  • [ ] YML for CI pipeline for new test projects
  • [ ] YML for signed pipeline
• [ ] Documentation updated: If checked, please file a pull request on our docs repo 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:

    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:

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.

[vanzue]

/azp run

[azure-pipelines[bot]]

Azure Pipelines successfully started running 1 pipeline(s).