Closed Bug 1385666 Opened 7 years ago Closed 7 years ago

phpBB(BBcode/HTML view) show wrong position conversion candidates Japanese IME.

Categories

(Core :: DOM: UI Events & Focus Handling, defect)

x86_64
Windows 10
defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla57
Tracking Status
firefox-esr52 --- unaffected
firefox54 --- unaffected
firefox55 --- unaffected
firefox56 --- verified
firefox57 --- verified

People

(Reporter: kuroweb, Assigned: smaug)

References

()

Details

(Keywords: inputmethod, multiprocess)

Attachments

(4 files)

Attached image phpbb_japanese_ime.gif
Build identifier: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:56.0) Gecko/20100101 Firefox/56.0

Steps to reproduce.
1.setting new profile.
2.open MozillaZine Forum https://forums.mozillazine.jp/
3.open "新しいトピック"(new topic), and switch to enable "bbcode/HTML view"(default).
4.turn on Japanese IME.
5.type to Japanese characters.

Actual results:
show wrong position conversation candidate.

Expected results.
show correct position.

Other browser results:
Google Chrome 60.0.3112.78(Official Build)64bit is fine.
Microsoft Edge 40.15063.0.0 is fine.

This bug does not occur in the phpbb demo forum.
https://www.phpbb.com/demo/
Component: General → Widget: Win32
Keywords: inputmethod
Product: Firefox → Core
I can reproduce the problem on Nightly56.0a1 windows10 with e10s.

Regression window:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=c1983642406a1ad7768d5d2363e521c11b4f1b2a&tochange=7eea0e7fbff9b6ccb5b6e9400b1278ec58dbca6b

Regressed by:
7eea0e7fbff9	Olli Pettay — Bug 1375491, make child process to cache ime properties only at animation tick time, r=masayuki

@:smaug,
Your patch seems to cause the problem, can you look this?
Blocks: 1375491
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(bugs)
Keywords: multiprocess
The symptom is really similar to bug 1380240. I hope, they are related...
Attached file simple test case
(In reply to Masayuki Nakano [:masayuki] (JST, +0900) from comment #3)
> The symptom is really similar to bug 1380240. I hope, they are related...

As long as I check HTML via inspector, it uses iframe.  So I believe this is same issue.
What should one do with the testcase?
Flags: needinfo?(m_kato)
- Step
1. focus iframe's box
2. Turn on Microsoft IME
3. Type [a] key to input 'あ'

- Result
no character in iframe's box.

- Expected Result
'あ' is shown in iframe's box like Firefox 54/55.
Flags: needinfo?(m_kato)
Hmm, on linux candidate window is always at wrong position :/
I mean even in release builds of FF.
(but that is a separate issue. I can reproduce comment 7 on linux too)
data:text/html,<script>onload = function() { document.designMode = "on"; } </script>

seems to show the issue too.
Flags: needinfo?(bugs)
Assignee: nobody → bugs
Component: Widget: Win32 → Event Handling
I wasn't going to make all the cases async, only the case when ESM tries to flush. In particular, IMEStateManager::OnFocusInEditor needs to be sync.

I'll try to figure out some way to test this, but hopefully we could land this sort-of-partial backout asap.
Attachment #8893091 - Flags: review?(masayuki)
Comment on attachment 8893091 [details] [diff] [review]
ime_less_async.diff

Nice. They must not make users feel performance regression with this change.
Attachment #8893091 - Flags: review?(masayuki) → review+
(In reply to Olli Pettay [:smaug] from comment #12)
> I'll try to figure out some way to test this, but hopefully we could land
> this sort-of-partial backout asap.

I think that immediately after designMode is set to "on", synthesize query events return error without the patch since ContentCacheInParent doesn't have content cache of the remote process. According to the warnings of debug build, we can test this with two ways:

1. use synthesizeQuery*() of EventUtils.js.
2. use nsITextInputProcessorCallback and check selection change notification after dispatching arrow key events.

Perhaps, #1 is easier.

However, there are no tests which synthesize events in the main process and focused editor is in a remote process yet.
Pushed by opettay@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/fc490a4565fc
ensure layout is flushed when editor gets focus, r=masayuki
Comment on attachment 8893091 [details] [diff] [review]
ime_less_async.diff

Approval Request Comment
[Feature/Bug causing the regression]: bug 1375491
[User impact if declined]: Bad IME handling. Inputting Japanese etc. may not work
[Is this code covered by automated tests?]: Not yet, and I'm basically backing out part of bug 1375491
[Has the fix been verified in Nightly?]: Now yet
[Needs manual test from QE? If yes, steps to reproduce]: See the testcase in thsi bug
[List of other uplifts needed for the feature/fix]: NA
[Is the change risky?]: Shouldn't be risky
[Why is the change risky/not risky?]: backing out part of the problematic patch.
[String changes made/needed]: NA
Attachment #8893091 - Flags: approval-mozilla-beta?
https://hg.mozilla.org/mozilla-central/rev/fc490a4565fc
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Hi Brindusa, could you help find someone to verify if this issue was fixed as expected on the latest Nightly build?
Flags: needinfo?(brindusa.tot)
Comment on attachment 8893091 [details] [diff] [review]
ime_less_async.diff

Partial backout, IME fix, let's take this for beta 2.
Attachment #8893091 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Flags: qe-verify+
I managed to reproduce the issue on an older Nightly 56.0a1 build (2017-07-29).
Verified as fixed on the latest Nightly build 57.0a1 (2017-08-10) on Windows 10.
Status: RESOLVED → VERIFIED
Flags: needinfo?(brindusa.tot)
I was able to reproduce the initial issue using old Nightly from 2017-07-30 and verified that the issue is fixed using Firefox 56 beta 12 on Windows 10 64bit and Ubuntu 16.04 32bit.
Flags: qe-verify+
Component: Event Handling → User events and focus handling
You need to log in before you can comment on or make changes to this bug.