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

RESOLVED FIXED in Firefox 50

Status

()

P2
normal
RESOLVED FIXED
3 years ago
2 years ago

People

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

Tracking

({regression})

48 Branch
mozilla51
x86_64
Windows 10
regression
Points:
---

Firefox Tracking Flags

(firefox48 wontfix, firefox49 wontfix, firefox50 fixed, firefox51 fixed)

Details

(Whiteboard: tpi:+)

Attachments

(3 attachments)

(Reporter)

Description

3 years ago
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).
(Reporter)

Updated

3 years ago
OS: Unspecified → Windows 10
Hardware: Unspecified → x86_64
Here's regression range:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=29022393615224517283b16adae938128f75e27b&tochange=eb7c36e2ef5d48262bc8566da9ea37623e7d0883

There are 4 bugs but only bug 1137561 touched windows code.
Status: UNCONFIRMED → NEW
Component: Untriaged → Widget: Win32
Ever confirmed: true
Keywords: inputmethod, regression
Product: Firefox → Core
See Also: → bug 1137561
Odd... We received WM_CHAR messages as is. I guess that I've destroyed dead key handling in bug 1137561.
Blocks: 1137561
Keywords: inputmethod
See Also: 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:+

Updated

3 years ago
status-firefox48: --- → affected
status-firefox49: --- → affected
status-firefox50: --- → affected
status-firefox51: --- → affected
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.
status-firefox48: affected → wontfix
status-firefox49: affected → fix-optional
Version: 49 Branch → 48 Branch
(Reporter)

Comment 10

3 years ago
Yes, these builds work. Thank you very much!
Большое спасибо! :)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 13

3 years ago
mozreview-review
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 14

3 years ago
mozreview-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+

Comment 15

3 years ago
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?

Comment 18

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/b1f40b39b2b8
https://hg.mozilla.org/mozilla-central/rev/ecf37ffff913
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-firefox51: affected → fixed
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+

Comment 22

2 years ago
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)
status-firefox49: fix-optional → wontfix
Flags: needinfo?(cupermountain)
You need to log in before you can comment on or make changes to this bug.