Closed Bug 1336331 Opened 3 years ago Closed 3 years ago

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

Categories

(Core :: Widget: Win32, defect)

All
Windows
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla54
Tracking Status
firefox51 --- wontfix
firefox52 --- fixed
firefox53 --- fixed
firefox54 --- fixed

People

(Reporter: masayuki, Assigned: masayuki)

References

Details

(Keywords: crash)

Crash Data

Attachments

(1 file)

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.
The coming patch depends on bug 1336322.
Assignee: nobody → masayuki
Status: NEW → ASSIGNED
Depends on: 1336322
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+
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
https://hg.mozilla.org/mozilla-central/rev/3504f8cdd850
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
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+
You need to log in before you can comment on or make changes to this bug.