Closed Bug 1297985 Opened 8 years ago Closed 8 years ago

The letter 'щ' can not be entered when using the Windows 10 Russian - Mnemonic keyboard layout.

Categories

(Core :: Widget: Win32, defect, P2)

48 Branch
x86_64
Windows 10
defect

Tracking

()

RESOLVED FIXED
mozilla51
Tracking Status
firefox48 --- wontfix
firefox49 --- wontfix
firefox50 --- fixed
firefox51 --- fixed

People

(Reporter: dan.isac.91, Assigned: masayuki)

References

Details

(Keywords: regression, Whiteboard: tpi:+)

Attachments

(3 files)

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:49.0) Gecko/20100101 Firefox/49.0 Build ID: 20160822111414 Steps to reproduce: 1. Use the Russian - Mnemonic keyboar layout on Windows 8/8.1/10. 2. Open Firefox, and try typing the letter 'щ'. This letter should appear after you press the 's' key, and then the 'c' key. Actual results: Instead of the letter 'щ' appearing, the result is ’сц’. Expected results: One letter 'щ' is expected to be appear, instead of 'сц’ (2 letters).
OS: Unspecified → Windows 10
Hardware: Unspecified → x86_64
Status: UNCONFIRMED → NEW
Component: Untriaged → Widget: Win32
Ever confirmed: true
Product: Firefox → Core
Attached file log (deleted) —
Odd... We received WM_CHAR messages as is. I guess that I've destroyed dead key handling in bug 1137561.
Okay, perhaps, I understand the cause of this bug. The regression point is this patch: https://bugzilla.mozilla.org/attachment.cgi?id=8720263&action=diff In this patch, when mCommittedCharsAndModifiers is longer than 1, we made NativeKey dispatch keypress events without WM_CHAR messages. However, this hits a hidden bug of KeyboardLayout. KeyboardLayout thinks that the key combination causes 2 characters "сц" but WM_CHAR comes only for "щ" but it's ignored. So, we need to investigate the cause of this bug in KeyboardLayout class. Fortunately, I hit this assertion: https://dxr.mozilla.org/mozilla-central/rev/01748a2b1a463f24efd9cd8abad9ccfd76b037b8/widget/windows/KeyboardLayout.cpp#2998 So, there is a bug around here.
Assignee: nobody → masayuki
Status: NEW → ASSIGNED
Hmm, according to the debugger, KeyboardLayout marks both "s" and "c" are dead key. And when dead key pressed twice, it assumes that the result is always both characters which are produced when each dead key pressed twice. So, KeyboardLayout might not work with the keyboard layout, sigh...
Priority: -- → P2
Whiteboard: tpi:+
Looks like this regressed in 48. We could still take a patch in 49 in late beta (i.e. very soon) if it seems worth the risk.
Version: 49 Branch → 48 Branch
Yes, these builds work. Thank you very much! Большое спасибо! :)
Comment on attachment 8788017 [details] Bug 1297985 part.1 Log KeybordLayout::LoadLayout() to help developers to understand what data is created https://reviewboard.mozilla.org/r/76548/#review74748
Attachment #8788017 - Flags: review?(m_kato) → review+
Comment on attachment 8788018 [details] Bug 1297985 part.2 KeyboardLayout should handle a composite character produced by 2 dead keys https://reviewboard.mozilla.org/r/76550/#review74796
Attachment #8788018 - Flags: review?(m_kato) → review+
Pushed by masayuki@d-toybox.com: https://hg.mozilla.org/integration/autoland/rev/b1f40b39b2b8 part.1 Log KeybordLayout::LoadLayout() to help developers to understand what data is created r=m_kato https://hg.mozilla.org/integration/autoland/rev/ecf37ffff913 part.2 KeyboardLayout should handle a composite character produced by 2 dead keys r=m_kato
Comment on attachment 8788017 [details] Bug 1297985 part.1 Log KeybordLayout::LoadLayout() to help developers to understand what data is created Approval Request Comment [Feature/regressing bug #]: Bug 1137561 [User impact if declined]: Users cannot type some characters with specific keyboard layout if the characters are composited by 2 dead keys. [Describe test coverage new/current, TreeHerder]: Landed on m-c. [Risks and why]: Low, because this only adds logging code of KeyboardLayout class. [String/UUID change made/needed]: Nothing.
Attachment #8788017 - Flags: approval-mozilla-aurora?
Comment on attachment 8788018 [details] Bug 1297985 part.2 KeyboardLayout should handle a composite character produced by 2 dead keys Approval Request Comment [Risks and why]: Mid. This patch changes dead key handling code. KeyboardLayout hasn't supported such key sequence, 1st key is a dead key and 2nd key is a dead key and the key sequence composites a character instead of inputting two keys' characters. Due to the change of bug 1137561, NativeKey class started to use KeyboardLayout class's mapping table in such cases. So, this was a hidden bug before that. The risk isn't low (could have some regressions of dead key handling), but the symptom is disaster. Therefore, I believe that it's worthwhile to uplift these patches. [String/UUID change made/needed]: Nothing.
Attachment #8788018 - Flags: approval-mozilla-aurora?
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
Hello Dan, could you please verify this issue is fixed as expected on a latest Nightly build? Thanks!
Comment on attachment 8788017 [details] Bug 1297985 part.1 Log KeybordLayout::LoadLayout() to help developers to understand what data is created Recent regression since 48, has stabilized in Nightly for 3 days, Aurora50+
Attachment #8788017 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Attachment #8788018 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Hi, im still having this issue
(In reply to cupermountain from comment #22) > Hi, > im still having this issue Did you tested with Beta, Developer Edition or Nightly?
Flags: needinfo?(dan.isac.91) → needinfo?(cupermountain)
Flags: needinfo?(cupermountain)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: