User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.11; rv:48.0) Gecko/20100101 Firefox/48.0 Build ID: 20160606200529 Steps to reproduce: - Long press a key which has an accent menu (like "e") in a text field - Select an accentuated character using a number key (no issue if using arrow keys or mouse to select the accentuated character) Actual results: - The accent menu disappears (expected) - The character doesn't get accentuated (not expected) - The character gets selected (not expected) Expected results: - The accent menu disappears - The character is replaced by the selected accentuated character (like "e" > "é") Un e

Hi, I tested on Mac OS X 10.11 with Firefox Nightly 50.0a1 (2016-06-12) and Firefox 47 release. I can reproduce the issue on Nightly but I can't reproduce it on release 47. I think this is a regression. I will come back with a regression range. Please try Firefox 47 release and see if you can reproduce this issue.

I can't reproduce it either on the release version of 47.0. I can reproduce it though on 48.0b1 (as reported above) as well as 49.0a2 2016-06-12 (Dev Edition)

I did a regression and here are the results: Last good revision: 29022393615224517283b16adae938128f75e27b First bad revision: eb7c36e2ef5d48262bc8566da9ea37623e7d0883 Pushlog: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=29022393615224517283b16adae938128f75e27b&tochange=eb7c36e2ef5d48262bc8566da9ea37623e7d0883

The push contained fixes for 5 bugs: bug 1137572, 1137563, 1137565, 1137561. I'm guessing that one of the patches in bug 1137563 caused this.

It must be so. Taking. [Tracking Requested - why for this release]: Serious regression of bug 1137563 according to comment 3. And it's impossible to back them out since a lot of bug fixes are based on the landing.

TextEventDispatcher::MaybeDispatchKeypressEvents() dispatches keypress events with passed event's mKeyNameIndex and mKeyValue values. I.e., setting mCharCode doesn't make sense in this case. Similarly, mKeyCode value is also ignored (overwritten by 0) if it's printable key's key event (mKeyNameIndex == KEY_NAME_INDEX_USE_STRING). Review commit: https://reviewboard.mozilla.org/r/59380/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/59380/

Comment on attachment 8762993 [details] Bug 1279170 TextInputHandler::InsertText() should set keypress event's .key value property when it replaces specified range with a character https://reviewboard.mozilla.org/r/59380/#review56458

Pushed by masayuki@d-toybox.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/4e16558233f9 TextInputHandler::InsertText() should set keypress event's .key value property when it replaces specified range with a character r=m_kato

Comment on attachment 8762993 [details] Bug 1279170 TextInputHandler::InsertText() should set keypress event's .key value property when it replaces specified range with a character Approval Request Comment [Feature/regressing bug #]: Important regression of bug 1137563. [User impact if declined]: Cannot input accented characters with long press of a key on OS X. And some IME might use same path for inputting a character without key events. [Describe test coverage new/current, TreeHerder]: Landed on m-c. [Risks and why]: Low because it's really special path. Cocoa calls InputText() with replacing range when user does NOT press a key. In such case, the inserting string is only one character, user meet this bug. In other words, most work of IMEs won't meet this bug. [String/UUID change made/needed]: Nothing.

Tracking 50+ for this regression which is now fixed.

I verified this on Mac OS X 10.10 FF Nightly 50.0a1(2016-06-20) and everything works as expected. I can't reproduce this issue.

Comment on attachment 8762993 [details] Bug 1279170 TextInputHandler::InsertText() should set keypress event's .key value property when it replaces specified range with a character Fix a major user facing regression, taking it in aurora & beta Should be in 48 beta 3

Could we have some automated tests for this so that we don't regress?

https://hg.mozilla.org/releases/mozilla-aurora/rev/5337f17b3c1f but has problems on uplift to beta: grafting 350775:5337f17b3c1f "Bug 1279170 - TextInputHandler::InsertText() should set keypress event's .key value property when it replaces specified range with a character. r=m_kato, a=sylvestre" (tip) merging widget/cocoa/TextInputHandler.mm warning: conflicts while merging widget/cocoa/TextInputHandler.mm! (edit, then use 'hg resolve --mark') abort: unresolved conflicts, can't continue (use 'hg resolve' and 'hg graft --continue')

(In reply to Sylvestre Ledru [:sylvestre] from comment #15) > Could we have some automated tests for this so that we don't regress? Unfortunately, no. We don't have a way to emulate the behavior with our testing API. (In reply to Carsten Book [:Tomcat] from comment #16) > https://hg.mozilla.org/releases/mozilla-aurora/rev/5337f17b3c1f > > but has problems on uplift to beta: > > grafting 350775:5337f17b3c1f "Bug 1279170 - TextInputHandler::InsertText() > should set keypress event's .key value property when it replaces specified > range with a character. r=m_kato, a=sylvestre" (tip) > merging widget/cocoa/TextInputHandler.mm > warning: conflicts while merging widget/cocoa/TextInputHandler.mm! (edit, > then use 'hg resolve --mark') > abort: unresolved conflicts, can't continue > (use 'hg resolve' and 'hg graft --continue') Oh, okay, I'll check it and post a patch for beta tomorrow.

Here is a patch for beta. Bug 1254755 which renamed members of WidgetKeyboardEvent (appending "m" prefix) is the cause of the conflict. Therefore, this patch doesn't change any logic.

(In reply to Masayuki Nakano [:masayuki] (Mozilla Japan) (not in London) from comment #18) > Created attachment 8764085 [details] [diff] [review] > Patch for beta (coflict reason is bug 1254755, so, not changed anything > except member names of WidgetKeyboardEvent) > > Here is a patch for beta. Bug 1254755 which renamed members of > WidgetKeyboardEvent (appending "m" prefix) is the cause of the conflict. > Therefore, this patch doesn't change any logic. thanks landed!

I verified this on Mac OS X 10.10 on Firefox Aurora 49.0a2 (2016-06-29) and Firefox Beta 48.0b4, I couldn't reproduce the issue.