If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

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

RESOLVED FIXED in Firefox 50

Status

()

Core
Widget: Win32
P2
normal
RESOLVED FIXED
a year ago
9 months ago

People

(Reporter: Dan Isac, 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:+)

MozReview Requests

()

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

Attachments

(3 attachments)

(Reporter)

Description

a year 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

a year 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
Created attachment 8784784 [details]
log
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

a year 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
https://treeherder.mozilla.org/#/jobs?repo=try&revision=e7024af6a2e3
https://treeherder.mozilla.org/#/jobs?repo=try&revision=25f795804652
Could you test one of these build?
x86: https://archive.mozilla.org/pub/firefox/try-builds/masayuki@d-toybox.com-7c524eeb01fded6a519a6577c0e3712a3cb36cd1/try-win32/firefox-51.0a1.en-US.win32.zip
x86-64: https://archive.mozilla.org/pub/firefox/try-builds/masayuki@d-toybox.com-7c524eeb01fded6a519a6577c0e3712a3cb36cd1/try-win64/firefox-51.0a1.en-US.win64.zip
Flags: needinfo?(dan.isac.91)
(Reporter)

Comment 10

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

Comment 13

a year 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

a year 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

a year 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

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/b1f40b39b2b8
https://hg.mozilla.org/mozilla-central/rev/ecf37ffff913
Status: ASSIGNED → RESOLVED
Last Resolved: a year 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+

Updated

a year ago
Attachment #8788018 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+

Comment 21

a year ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-aurora/rev/0ebd731f42bb
https://hg.mozilla.org/releases/mozilla-aurora/rev/befed59939f7
status-firefox50: affected → fixed

Comment 22

11 months 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.