Closed Bug 1468917 Opened 6 years ago Closed 6 years ago

When the ATOK2016 candidate window pops up, it flickers if browser in maximized size mode

Categories

(Core :: Widget: Win32, defect)

Product:
Core
Component:
Widget: Win32
Platform:
x86_64
Windows 10
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla63
Tracking Flags:
Tracking Status
firefox-esr52 --- wontfix
firefox-esr60 --- wontfix
firefox60 --- wontfix
firefox61 --- wontfix
firefox62 --- fixed
firefox63 --- fixed

People

(Reporter: alice0775, Assigned: masayuki)

Details

(5 keywords)

Attachments

(2 files)

Bug 1468917 - part 1: Make TSFTextStore not create native caret if ATOK 2016 is active
59 bytes, text/x-review-board-request
m_kato
: review+
lizzard
: approval-mozilla-beta+
Details
Bug 1468917 - part 2: TSFTextStore::GetTextExt() shouldn't return TS_E_NOLAYOUT when ATOK retrieves text rects *in* the composition string
59 bytes, text/x-review-board-request
m_kato
: review+
lizzard
: approval-mozilla-beta+
Details 
Reproducible: always

Steps to Reproduce:
1. Maximize browser
2. Open data:text/html,<input> or any input field and select it
3. IME on
4. Type someting
5. Press Conversion key until candidate windou popup


Actual Results:
When the ATOK2016 candidate window pops up.
It appears at corner of screen for a moment. And then it moves to the correct position.

Expected Results:
Not flicker


Disabling e10s seems to fix the problem.

Other Browser does not have such annoying problem.
Keywords: inputmethod
Summary: When the ATOK2016 candidate window pops up, it flickers → When the ATOK2016 candidate window pops up, it flickers if browser in maximized size mode
Hmm, cannot reproduce.

1. Is it recent regression?
2. Do you use InsiderPreview build in Fast ring?
3. Did you change intl.tsf.*?
4. Do you enable ATOKインサイト (設定 -> 入力・変換 -> 変換補助 -> 一時文書学習候補を表示する) and enable a11y feature of Firefox?
5. Do you see composition string when candidate window is not positioned correctly?

Sounds like the symptom is, when you need the candidate window, remote process hasn't notified the main process of content information such as character rectangles of composition string, caret position, etc. So, this could occur when the child process too busy or you typed really fast like machine. Anyway, if so, composition string shouldn't be shown in the editor since if remote process received a composition event at least once, remote process should've already sent layout information to the main process.
Flags: needinfo?(alice0775)
(In reply to Masayuki Nakano [:masayuki] (JST, +0900) from comment #1)
> Hmm, cannot reproduce.
> 
> 1. Is it recent regression?

No. at least it is replroduced on Firefox45 if e10s forcibly enabled

> 2. Do you use InsiderPreview build in Fast ring?

No. version 1803 os build 17134.112

> 3. Did you change intl.tsf.*?

No.

> 4. Do you enable ATOKインサイト (設定 -> 入力・変換 -> 変換補助 -> 一時文書学習候補を表示する) and enable
> a11y feature of Firefox?

Yes.

Disabled a11y does not help.
Disabled ATOKインサイト does not help.

> 5. Do you see composition string when candidate window is not positioned
> correctly?
> 

Yes. the composition string position seems to be ok.
Flags: needinfo?(alice0775)
Interestingly, 
this problem only occurs in maximized browser.
Not occur in normal size-mode and fullscreen size-mode.
However even in normal size-mode, the problem occurs if the top-left corner of the browser is outside of monitor screen.
Alice-san:

Thank you for the information, and sorry for the delay to reply due to the business trip.

I'm still trying to reproduce this bug, but not yet reproducible.

According to comment 3, sounds like a bug around multi-monitor.

Do you use 2 or more displays? If yes, do you use GPU driver specific feature like treating all displays as part of a desktop?

Do you use virtual desktop feature of Windows 10? Or something similar software of 3rd party?
Flags: needinfo?(alice0775)
(In reply to Masayuki Nakano [:masayuki] (JST, +0900) from comment #4)
> Do you use 2 or more displays? If yes, do you use GPU driver specific
> feature like treating all displays as part of a desktop?

No. I use only 1 monitor.


> 
> Do you use virtual desktop feature of Windows 10? Or something similar
> software of 3rd party?

No.



I can also reproduce with new account of Windows10 Home x64 Ver1803 Build17134.112(i.e. default setting of windows and Atok2016).
Flags: needinfo?(alice0775)
Okay, it seems that I succeeded to reproduce this bug on VM which does not have 2nd display nor 2nd virtual desktop.

As far as I've tested, this looks like a bug of hack of ATOK. If we don't create native caret for ATOK (with disabling intl.tsf.hack.atok.create_native_caret), I cannot reproduce this bug. How about you, Alice-san?
Flags: needinfo?(alice0775)
(In reply to Masayuki Nakano [:masayuki] (JST, +0900) from comment #6)
> Okay, it seems that I succeeded to reproduce this bug on VM which does not
> have 2nd display nor 2nd virtual desktop.
> 
> As far as I've tested, this looks like a bug of hack of ATOK. If we don't
> create native caret for ATOK (with disabling
> intl.tsf.hack.atok.create_native_caret), I cannot reproduce this bug. How
> about you, Alice-san?


I confirmed that I cannot reproduce that problem anymore by setting intl.tsf.hack.atok.create_native_caret=false.
Flags: needinfo?(alice0775)
Thank you. Perhaps, we should stop doing the hack for ATOK 2016 and later since I don't see any problem with disabling it but I see side-effect of this hack mentioned in the following article.
https://d-toybox.com/studio/weblog/show.php?id=2018060200#ATOK2017

FYI: I don't reproduce this bug with ATOK 2017 nor ATOK 2015.
Assignee: nobody → masayuki
Status: NEW → ASSIGNED

(In reply to Alice0775 White from comment #7)
> (In reply to Masayuki Nakano [:masayuki] (JST, +0900) from comment #6)
> > Okay, it seems that I succeeded to reproduce this bug on VM which does not
> > have 2nd display nor 2nd virtual desktop.
> > 
> > As far as I've tested, this looks like a bug of hack of ATOK. If we don't
> > create native caret for ATOK (with disabling
> > intl.tsf.hack.atok.create_native_caret), I cannot reproduce this bug. How
> > about you, Alice-san?
> 
> 
> I confirmed that I cannot reproduce that problem anymore by setting
> intl.tsf.hack.atok.create_native_caret=false.

I think the side-effect is more annoying than this bug, because, the flickering occurs every next candidate is selected with arrowkey in the candidate window.

I think that WONTFIX is best for this bug unless the side effect is resolved.
No, the side-effect appears when we *do* the hack, and not doing the hack *fixes* this bug.
Ah, but another side-effect appears without the hack. That is the candidate window may appear at first clause even when second or later clause is being converted. I'll try to look for preventing this issue.
FYI, The try build in comment#13 works as expected(no flicker) on win64 opt and win32 opt build with/without e10s on Windows10.
Thank you for your test. I also don't see any problem with the build even with ATOK 2015, ATOK 2016, ATOK 2017 and the latest ATOK of ATOK Passport. I think the patches are enough safe.
Comment on attachment 8987045 [details]
Bug 1468917 - part 1: Make TSFTextStore not create native caret if ATOK 2016 is active

https://reviewboard.mozilla.org/r/252288/#review259088
Attachment #8987045 - Flags: review?(m_kato) → review+
Comment on attachment 8987046 [details]
Bug 1468917 - part 2: TSFTextStore::GetTextExt() shouldn't return TS_E_NOLAYOUT when ATOK retrieves text rects *in* the composition string

https://reviewboard.mozilla.org/r/252290/#review259090
Attachment #8987046 - Flags: review?(m_kato) → review+
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/6b99a97dc402
part 1: Make TSFTextStore not create native caret if ATOK 2016 is active r=m_kato
https://hg.mozilla.org/integration/autoland/rev/e3179eede59d
part 2: TSFTextStore::GetTextExt() shouldn't return TS_E_NOLAYOUT when ATOK retrieves text rects *in* the composition string r=m_kato
https://hg.mozilla.org/mozilla-central/rev/6b99a97dc402
https://hg.mozilla.org/mozilla-central/rev/e3179eede59d
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox63: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Masayuki, is it ok to let this change ride to release with 63, or do you think it is good for uplift to 62? Thanks!
status-firefox62: affected → fix-optional
Flags: needinfo?(masayuki)
(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #22)
> Masayuki, is it ok to let this change ride to release with 63, or do you
> think it is good for uplift to 62? Thanks!

I was thinking as the latter. However, the fix improves UX of the latest version of ATOK and the risk is really low. Sure. I'll request to uplift.
Flags: needinfo?(masayuki)
Comment on attachment 8987045 [details]
Bug 1468917 - part 1: Make TSFTextStore not create native caret if ATOK 2016 is active

Approval Request Comment
[Feature/Bug causing the regression]:
Regression of e10s.

[User impact if declined]:
ATOK users see annoying flicker of candidate window.

[Is this code covered by automated tests?]:
No since we don't have any automated test framework for native IME handler.

[Has the fix been verified in Nightly?]:
Yes.

[Needs manual test from QE? If yes, steps to reproduce]: 
No, ATOK is 3rd party's IME, not free.  So, it's difficult to create environment.

[List of other uplifts needed for the feature/fix]:
The following patch.

[Is the change risky?]:
No.

[Why is the change risky/not risky?]:
Background:
ATOK is a 3rd party's Japanese IME since many years ago. We've create Windows' native caret for some Chinese IMEs when Win9x, 2k and XP since they referred native caret position to show their reading window. I'm not sure the exact version number, ATOK started to refer the native caret position only when active window is Gecko. Therefore, for supporting old ATOK (2011 - 2015), we need to keep native caret even after we supported TSF on Vista and later.

On the other hand, at ATOK 2016, they probably rewrite the code to decide their candidate window. They still refer native caret if there is, however, it includes at least this bug.  Additionally, if there is no native caret, they decide the position only with API of TSF.

So, we should drop ATOK 2016 from the blacklist of ATOK which require native caret so that this patch only do it.  Note that ATOK 2017 and "ATOK" (starting 2018, there is only subscription license. So, the year is dropped from the product name).

[String changes made/needed]:
No.
Attachment #8987045 - Flags: approval-mozilla-beta?
Comment on attachment 8987046 [details]
Bug 1468917 - part 2: TSFTextStore::GetTextExt() shouldn't return TS_E_NOLAYOUT when ATOK retrieves text rects *in* the composition string

Approval Request Comment
[Feature/Bug causing the regression]:
Regression of the previous patch for ATOK 2016 users.
Regression of bug 1273510 for ATOK 2017 and the latest ATOK users.

[User impact if declined]:
Candidate window of ATOK may dance around proper position when user is choosing a word from candidate window.

[Is this code covered by automated tests?]:
No.

[Has the fix been verified in Nightly?]:
Yes.

[Needs manual test from QE? If yes, steps to reproduce]:
No.

[List of other uplifts needed for the feature/fix]:
The previous patch.

[Is the change risky?]:
No.

[Why is the change risky/not risky?]:
When there is no native caret, ATOK decides candidate window position only with API of TSF. However, the API is still buggy in the latest Win10 (it'll be fixed on the next Win10 update at this fall). Therefore, until remote process finishes reflow for the new composition string, we need to notify ATOK of fake position rather than an error code to notify IME of waiting next layout change notification from us.

At fixing bug 1273510, I thought ATOK always queries rectangle of whole composition string when there is no native caret.  However, actually, ATOK may query rectangle between 2nd or later character and the last character of composition string.  In this case, we return error code so that ATOK confused.

This patch makes us not return the error code to ATOK if ATOK queries rectangle of *part* of composition string.

[String changes made/needed]:
No.

FYI: ATOK 2016 and later's share in Nightly build testers who input Japanese text is about 6%.
Attachment #8987046 - Flags: approval-mozilla-beta?
> FYI: ATOK 2016 and later's share in Nightly build testers who input Japanese text is about 6%.

Oops, I used wrong parameter, the correct number is about 11%.
Comment on attachment 8987045 [details]
Bug 1468917 - part 1: Make TSFTextStore not create native caret if ATOK 2016 is active

IME fix, verified in Nightly, let's uplift for beta 7.
Attachment #8987045 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #8987046 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: