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)
Tracking
()
RESOLVED
FIXED
mozilla51
People
(Reporter: dan.isac.91, Assigned: masayuki)
References
Details
(Keywords: regression, Whiteboard: tpi:+)
Attachments
(3 files)
(deleted),
text/plain
|
Details | |
(deleted),
text/x-review-board-request
|
m_kato
:
review+
ritu
:
approval-mozilla-aurora+
|
Details |
(deleted),
text/x-review-board-request
|
m_kato
:
review+
ritu
:
approval-mozilla-aurora+
|
Details |
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).
Comment 1•8 years ago
|
||
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
Assignee | ||
Comment 2•8 years ago
|
||
Assignee | ||
Comment 3•8 years ago
|
||
Odd... We received WM_CHAR messages as is. I guess that I've destroyed dead key handling in bug 1137561.
Assignee | ||
Updated•8 years ago
|
Blocks: 1137561
Keywords: inputmethod
Assignee | ||
Comment 4•8 years ago
|
||
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
Assignee | ||
Comment 5•8 years ago
|
||
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...
Updated•8 years ago
|
Priority: -- → P2
Whiteboard: tpi:+
Updated•8 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.
Updated•8 years ago
|
Version: 49 Branch → 48 Branch
Assignee | ||
Comment 7•8 years ago
|
||
Assignee | ||
Comment 8•8 years ago
|
||
Assignee | ||
Comment 9•8 years ago
|
||
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•8 years ago
|
||
Yes, these builds work. Thank you very much!
Большое спасибо! :)
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 13•8 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•8 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•8 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
Assignee | ||
Comment 16•8 years ago
|
||
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?
Assignee | ||
Comment 17•8 years ago
|
||
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•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/b1f40b39b2b8
https://hg.mozilla.org/mozilla-central/rev/ecf37ffff913
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+
Comment 21•8 years ago
|
||
bugherder uplift |
Comment 22•8 years ago
|
||
Hi,
im still having this issue
Assignee | ||
Comment 23•8 years ago
|
||
(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)
Updated•8 years ago
|
Flags: needinfo?(cupermountain)
You need to log in
before you can comment on or make changes to this bug.
Description
•