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

VERIFIED FIXED in Firefox 56

Status

()

defect
VERIFIED FIXED
2 years ago
2 months ago

People

(Reporter: kuroweb, Assigned: smaug)

Tracking

({inputmethod, multiprocess})

Trunk
mozilla57
x86_64
Windows 10
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox-esr52 unaffected, firefox54 unaffected, firefox55 unaffected, firefox56 verified, firefox57 verified)

Details

()

Attachments

(4 attachments)

Reporter

Description

2 years ago
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

Comment 1

2 years ago
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...
(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.
Assignee

Comment 6

2 years ago
What should one do with the testcase?
Assignee

Updated

2 years ago
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)
Assignee

Comment 8

2 years ago
Hmm, on linux candidate window is always at wrong position :/
Assignee

Comment 9

2 years ago
I mean even in release builds of FF.
Assignee

Comment 10

2 years ago
(but that is a separate issue. I can reproduce comment 7 on linux too)
Assignee

Comment 11

2 years ago
data:text/html,<script>onload = function() { document.designMode = "on"; } </script>

seems to show the issue too.
Flags: needinfo?(bugs)
Assignee

Updated

2 years ago
Assignee: nobody → bugs
Assignee

Updated

2 years ago
Component: Widget: Win32 → Event Handling
Assignee

Comment 12

2 years ago
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)
Assignee

Comment 13

2 years ago
remote: View your change here:
remote:   https://hg.mozilla.org/try/rev/75ab574282253c391e5235217f3962c23a412584
remote: 
remote: Follow the progress of your build on Treeherder:
remote:   https://treeherder.mozilla.org/#/jobs?repo=try&revision=75ab574282253c391e5235217f3962c23a412584
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.

Comment 16

2 years ago
Pushed by opettay@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/fc490a4565fc
ensure layout is flushed when editor gets focus, r=masayuki
Assignee

Comment 17

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

Comment 18

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/fc490a4565fc
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Duplicate of this bug: 1380240
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
Product: Core → Core
You need to log in before you can comment on or make changes to this bug.