[Win32] Do not set IME font if the font is disposed #2365
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
ShahzaibIbrahim
suggested changes
Aug 1, 2025
Contributor
ShahzaibIbrahim
left a comment
ShahzaibIbrahim
left a comment
There was a problem hiding this comment.
The changes look sound and adhere to previous state. I have just one comment.
Comment on lines
513
to
514
| { | ||
| hFont = font.isDisposed() ? 0 : SWTFontProvider.getFontHandle(font, getNativeZoom()); |
Contributor
There was a problem hiding this comment.
Shouldn't we check for font being disposed inside of the SWTFontProvider.getFontHandle and return 0 if its disposed? Wouldn't that be safer for all the consumers?
Contributor
There was a problem hiding this comment.
Hmm, yes, sounds good to me. That would solve this issue here as well and is equal to the original behavior: Is a font disposed return a 0 handle
arunjose696
force-pushed
the
arunjose696/368/FontDisposalIMEFont
branch
from
053451d to
0ddbf9f
Compare
…2323 Prior to recent changes that ensure that a font handle is always retrieved via the SWTFontProvider, the hFont variable in Caret::setIMEFont was retrieved directly from the font object (using font.handle). Since those recent changes, hFont is fetched via SWTFontProvider, which throws an exception if the font is disposed. Previously, when the font was disposed, hFont (font.handle) would be zero, and the method would fall back to using defaultFont for setting the IME font. Now, this fallback no longer works because the exception is thrown before the fallback can occur. This commit restores the intended behavior by setting hFont as zero if the font is disposed, preventing the exception. Fixes eclipse-platform#2323
HeikoKlare
force-pushed
the
arunjose696/368/FontDisposalIMEFont
branch
from
0ddbf9f to
1426edf
Compare
HeikoKlare
changed the title
HeikoKlare
approved these changes
Aug 3, 2025
Contributor
HeikoKlare
left a comment
HeikoKlare
left a comment
There was a problem hiding this comment.
The change is sound and fixes the reported issue.
I have adapted the PR with corrected formatting and slightly updated the commit message to fit to our recommendations.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Prior to PR #2062, the
hFontvariable inCaret::setIMEFontwas retrieved directly from thefontobject (usingfont.handle). Since #2602,hFontis fetched viaSWTFontProvider, which throws an exception if the font is disposed.Previously, when the font was disposed, hFont (
font.handle) would be zero, and the method would fall back to using defaultFont for setting the IME font. After #2602, this fallback no longer works because the exception is thrown before the fallback can occur.This commit restores the intended behavior by setting
hfontas zero if the font is disposed, preventing the exception.Fixes #2323
Steps to reproduce
Run the snippet provided in #2323 (comment).
Without this change, an exception will be thrown when the font is disposed.