Even there is WM_DEADCHAR in the queue, PeekMessage() may fail to remove WM_DEADCHAR message from the queue

RESOLVED FIXED in Firefox 52

Status

()

Core
Widget: Win32
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: masayuki, Assigned: masayuki)

Tracking

({crash})

Trunk
mozilla54
All
Windows
crash
Points:
---
Dependency tree / graph
Bug Flags:
qe-verify -

Firefox Tracking Flags

(firefox51 wontfix, firefox52 fixed, firefox53 fixed, firefox54 fixed)

Details

(crash signature)

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

https://crash-stats.mozilla.com/report/index/76427a6a-cd80-43d5-b5e8-d71002170201
> PeekMessage() failed to remove char message! 
> Handling message: { message=WM_KEYDOWN, virtual keycode=VK_BACK, repeat count=1, scancode=0x11, extended key=false, context code=false, previous key state=false, transition state=false, hwnd=0x700e6, InSendMessageEx()=ISMEX_NOSEND, 
> Found message: { message=WM_DEADCHAR, character code='㕲' (0x3572), repeat count=1, scancode=0x11, extended key=false, context code=false, previous key state=false, transition state=false, hwnd=0x700e6, 
> WM_NULL has been removed: 0, 
> Next key message in all windows: { message=WM_DEADCHAR, character code=NULL (0x0000), repeat count=1, scancode=0x11, extended key=false, context code=false, previous key state=false, transition state=false, hwnd=0x700e6, time=8450453, 
> Next message in all windows: { message=WM_DEADCHAR, character code=NULL (0x0000), repeat count=1, scancode=0x11, extended key=false, context code=false, previous key state=false, transition state=false, hwnd=0x700e6, time=8450453

This is the last case in crash reports which we reported. Oddly, KeyW causes 'Backspace' virtual keycode and caused WM_DEADCHAR with a Kanji character. Then, PeekMessage() failed to remove it from the queue. However, there is still WM_DEADCHAR existing in the queue. Additionally, only the WM_DEADCHAR's wParam is changed, i.e., caused by same key, caused on same window.

In this case, the char code becomes 0, so, we might be able to ignore only in this case, though.

I'm still thinking how we should handle this case.
status-firefox51: --- → wontfix
status-firefox52: --- → affected
status-firefox53: --- → affected
See Also: → bug 1336322
The coming patch depends on bug 1336322.
Assignee: nobody → masayuki
Status: NEW → ASSIGNED
Depends on: 1336322
Comment hidden (mozreview-request)

Comment 3

2 years ago
mozreview-review
Comment on attachment 8833292 [details]
Bug 1336331 NativeKey::GetFollowingCharMessage() should try to use GetMessage() when PeekMessage() failed to remove a char message from the queue and there is still existing a char message

https://reviewboard.mozilla.org/r/109550/#review110968
Attachment #8833292 - Flags: review?(m_kato) → review+

Comment 4

2 years ago
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/3504f8cdd850
NativeKey::GetFollowingCharMessage() should try to use GetMessage() when PeekMessage() failed to remove a char message from the queue and there is still existing a char message r=m_kato

Comment 5

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/3504f8cdd850
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox54: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Comment on attachment 8833292 [details]
Bug 1336331 NativeKey::GetFollowingCharMessage() should try to use GetMessage() when PeekMessage() failed to remove a char message from the queue and there is still existing a char message

Approval Request Comment
[Feature/Bug causing the regression]: Long standing bug which we don't handle odd keyboard message behavior of Indian keyboard utility.
[User impact if declined]: Some users around India may meet this crash at typing something. See bug 962140 comment 144 for the detail.
[Is this code covered by automated tests?]: No.
[Has the fix been verified in Nightly?]: No, because we're still not sure how to reproduce this bug but we have not gotten any regression reports.
[Needs manual test from QE? If yes, steps to reproduce]: No.
[List of other uplifts needed for the feature/fix]: Perhaps, this may depend on the patches for bug 1336080. (It will be fine if you land the uplifts from smaller bug number to larger number of this series of crash bugs.)
[Is the change risky?]: No.
[Why is the change risky/not risky?]: When we detect odd case, we have been crashing by ourselves. This patch avoids the crash in the specific case mentioned in comment 0. So, this doesn't change normal path.
[String changes made/needed]: No.

FYI: Nobody of Aurora/Nightly channel testers has met this crash. So, the odd keyboard utility users are using only Release and Beta. So, it does not make sense to wait 12 weeks for such users.
Attachment #8833292 - Flags: approval-mozilla-beta?
Attachment #8833292 - Flags: approval-mozilla-aurora?
Comment on attachment 8833292 [details]
Bug 1336331 NativeKey::GetFollowingCharMessage() should try to use GetMessage() when PeekMessage() failed to remove a char message from the queue and there is still existing a char message

crash fix for aurora and 52.0b4
Attachment #8833292 - Flags: approval-mozilla-beta?
Attachment #8833292 - Flags: approval-mozilla-beta+
Attachment #8833292 - Flags: approval-mozilla-aurora?
Attachment #8833292 - Flags: approval-mozilla-aurora+

Comment 8

2 years ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-aurora/rev/05fae22830c0
status-firefox53: affected → fixed

Comment 9

2 years ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-beta/rev/501a3fa83897
status-firefox52: affected → fixed
Flags: qe-verify-
You need to log in before you can comment on or make changes to this bug.