Skip to content

Fix message hooking regression#14789

Merged
michaelDCurran merged 5 commits into
nvaccess:masterfrom
LeonarddeR:detoursRegress
Apr 14, 2023
Merged

Fix message hooking regression#14789
michaelDCurran merged 5 commits into
nvaccess:masterfrom
LeonarddeR:detoursRegress

Conversation

@LeonarddeR

@LeonarddeR LeonarddeR commented Apr 5, 2023

Copy link
Copy Markdown
Collaborator

Link to issue number:

Follow up of #14759
Fixes #14784
Partially reverts 9b97de0

Summary of the issue:

At least one custom IME is reported to no longer work in editable controls within NVDA.

Description of user facing changes

Fix the regression

Description of development approach

We no longer use the background thread to handle cancellable messages, as it seems using the background thread causes IME messages to be discareded. This means a partial revert of 9b97de0. In fact, this pr removes the background thread but keeps the event based approach to cancel calls from NVDA's watchdog.

Testing strategy:

Known issues with pull request:

None known

Change log entries:

Add this pr and issue #14784 to the changelog entry for #14759

Code Review Checklist:

  • Pull Request description:
    • description is up to date
    • change log entries
  • Testing:
    • Unit tests
    • System (end to end) tests
    • Manual testing
  • API is compatible with existing add-ons.
  • Documentation:
    • User Documentation
    • Developer / Technical Documentation
    • Context sensitive help for GUI changes
  • UX of all users considered:
    • Speech
    • Braille
    • Low Vision
    • Different web browsers
    • Localization in other languages / culture than English
  • Security precautions taken.

@cary-rowen

Copy link
Copy Markdown
Contributor

I tested the latest build and #14784 is still reproducible.

@LeonarddeR

Copy link
Copy Markdown
Collaborator Author

Is this ime easily testable for people who do not understand Chinese?

@cary-rowen

Copy link
Copy Markdown
Contributor

Just need to install Sogou Pinyin input method, if the steps in #14784 are not easy to understand, please let me know, I am more than willing to provide further explanation.

@cary-rowen

Copy link
Copy Markdown
Contributor

Hi, I tested the latest build and #14784 is still reproducible.
For debugging purposes, would it be helpful if I could provide a virtual machine with NVDARemote installed for you to test?

@LeonarddeR

Copy link
Copy Markdown
Collaborator Author

I hadn't thought about that, I have a vm myself, so that'll work.

@LeonarddeR LeonarddeR changed the title Try to work around Sendmessage hooking regressions Fix message hooking regression Apr 11, 2023
@LeonarddeR

LeonarddeR commented Apr 11, 2023

Copy link
Copy Markdown
Collaborator Author

@cary-rowen The build that's currently being produced should fix your issue. Could you also confirm that #13335 is still no longer reproducible?

@cary-rowen

Copy link
Copy Markdown
Contributor

Yes, I tested, and the latest build fixes #14784 , thanks.

@LeonarddeR

Copy link
Copy Markdown
Collaborator Author

Thanks @cary-rowen. How about #13335?

@cary-rowen

Copy link
Copy Markdown
Contributor

Hi @LeonarddeR

#13335 can also be covered by this PR, very nice.

Thanks

@LeonarddeR LeonarddeR marked this pull request as ready for review April 12, 2023 12:25
@LeonarddeR LeonarddeR requested a review from a team as a code owner April 12, 2023 12:25
@LeonarddeR LeonarddeR requested a review from seanbudd April 12, 2023 12:25
@LeonarddeR

Copy link
Copy Markdown
Collaborator Author

In that case, it's ready.

@cary-rowen

Copy link
Copy Markdown
Contributor

@seanbudd This is ready.

@seanbudd seanbudd requested review from michaelDCurran and removed request for seanbudd April 12, 2023 23:58
@michaelDCurran michaelDCurran merged commit 9fe5207 into nvaccess:master Apr 14, 2023
@nvaccessAuto nvaccessAuto added this to the 2023.2 milestone Apr 14, 2023
@LeonarddeR LeonarddeR deleted the detoursRegress branch August 23, 2025 06:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Regression of #14759: Unable to use Sogou Pinyin to input Chinese characters in NVDA's editable controls

4 participants