1419285 - IME candidate window position is always top-left when using Hatena Bookmark Web Extension

Status: VERIFIED FIXED

(Core :: IPC, defect)

Description

[Makoto Kato [:m_kato]]

I don't know whether this is regression.  When using Hatena bookmark extension, IME candidate window isn't shown on correct position.

I tested on Windows 10 Fall creator update with Google Japanese Input, ATOK 2017 , Microsoft IME (Japanese and Korean).

Comment 1

[Makoto Kato [:m_kato]]

- step
1. Install Hatena Bookmark addon
2. Click Bookmark button
3. Input any comment on textbox

- result
IME candidate window position is always top-left

Comment 2

[Makoto Kato [:m_kato]]

Maybe, IME won't work on all web extension since web extension works on own process.

Comment 3

[Makoto Kato [:m_kato]]

[Tracking Requested - why for this release]:
IME doesn't work on panel in multiple Web Extensions.

Original reporter (not me!) says IME doesn't work on the following addons at least.
- https://addons.mozilla.org/ja/firefox/addon/todo/
- https://addons.mozilla.org/ja/firefox/addon/hatena-bookmark/

Comment 4

[Makoto Kato [:m_kato]]

Workaround is extensions.webextensions.remote=false

Comment 5

[YUKI "Piro" Hiroshi]

My addon "Second Search" also has same problem.
https://addons.mozilla.org/firefox/addon/second-search/

Comment 6

[Makoto Kato [:m_kato]]

(In reply to YUKI "Piro" Hiroshi from comment #5)
> My addon "Second Search" also has same problem.
> https://addons.mozilla.org/firefox/addon/second-search/

Yes, I believe that IME doesn't work on all web extension's panel.

Comment 7

[Makoto Kato [:m_kato]]

Since Web extension's popup is remote, TabParent will use popup's widget for NotifyIMEFocus (by web extension oop support).  It isn't toplevel widget, so threadMgr->AssociateFocus doesn't enable edit session correctly.

Humm...

Comment 8

[Gerry Chang [:gchang]]

Track 58+/59+ as IME doesn't work on panel in multiple Web Extensions.

Comment 9

[Makoto Kato [:m_kato]]

changing to valid component.

This also occurs on macOS with Web Extension OOP.  TabParent's GetWidget doesn't return toplevel widget.  We should uss GetDocWidget() for IME.

Comment 10

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to Makoto Kato [:m_kato] from comment #9)
> changing to valid component.
> 
> This also occurs on macOS with Web Extension OOP.  TabParent's GetWidget
> doesn't return toplevel widget.  We should uss GetDocWidget() for IME.

Or, there should be GetWidgetForIME() and make it call nsPresContext::GetRootWidget(). IMEStateManager always use it for handling IME like: https://searchfox.org/mozilla-central/rev/a5d613086ab4d0578510aabe8653e58dc8d7e3e2/dom/events/IMEStateManager.cpp#1789,1801

Comment 11

[Makoto Kato [:m_kato]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=c31cb52944ed16d42d80121ebe80426c8d43ee5a

Comment 13

[Makoto Kato [:m_kato]]

Attachment: Bug 1419285 - Part 1. Calculate composition rect for remote XUL frame.

Out of process support for Web extension creates another XUL widget for it.
We should calculate correct offset for Web Extension's remote widget.

Review commit: https://reviewboard.mozilla.org/r/204160/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/204160/

Comment 14

[Makoto Kato [:m_kato]]

Attachment: Bug 1419285 - Part 2. IME message should post to correct widget.

After landing bug 1353060, TabParent::GetWidget()'s behaviour is changed.  We
should use GetDocWidget for IME.

Review commit: https://reviewboard.mozilla.org/r/204162/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/204162/

Comment 15

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Comment on attachment 8933216 [details]
Bug 1419285 - Part 1. Calculate composition rect for remote XUL frame.

https://reviewboard.mozilla.org/r/204160/#review209660

Comment 16

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Comment on attachment 8933217 [details]
Bug 1419285 - Part 2. IME message should post to correct widget.

https://reviewboard.mozilla.org/r/204162/#review209662

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/19831481168f
Part 1. Calculate composition rect for remote XUL frame. r=masayuki
https://hg.mozilla.org/integration/autoland/rev/ab629f447e6c
Part 2. IME message should post to correct widget. r=masayuki

Comment 18

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/19831481168f
https://hg.mozilla.org/mozilla-central/rev/ab629f447e6c

Comment 19

[Rockridge]

When I use the latest Nightly 59.0a1 (Build ID:20171201220040), IME works well on a web extension's pop-up panel. I've tested it with ATOK 2017 and Microsoft IME (Japanese).

Comment 20

[Ryan VanderMeulen [:RyanVM]]

Please request Beta approval on this when you get a chance.

Comment 21

[Makoto Kato [:m_kato]]

Comment on attachment 8933216 [details]
Bug 1419285 - Part 1. Calculate composition rect for remote XUL frame.

Approval Request Comment
[Feature/Bug causing the regression]:
bug 1357486

[User impact if declined]:
IME doesn't work on Web Extension's popup panel.

[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]: 
Yes if possible

Step
1. Install Microsoft IME on Windows
2. Install https://addons.mozilla.org/firefox/addon/second-search/
3. Click Second Search Icon by 2.'s extension.
4. Input any Japanese word in [Search with Keyword] editbox

Result
Composing string is inline, not top-left screen

[List of other uplifts needed for the feature/fix]:
N/A

[Is the change risky?]:
Low

[Why is the change risky/not risky?]:
Child widget that is remote process will be created by Web extensions only.  So it is affected by remote web extension only.

[String changes made/needed]:
N/A

Comment 22

[Gerry Chang [:gchang]]

Comment on attachment 8933216 [details]
Bug 1419285 - Part 1. Calculate composition rect for remote XUL frame.

Fix an issue that IME doesn't work on Web Extension's popup panel and was verified. Beta58+.

Comment 23

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/51aeac094956
https://hg.mozilla.org/releases/mozilla-beta/rev/17253790a570

Comment 24

[Oana Botisan, Desktop QA]

I tried to reproduce the bug both on Mac OS 10.11 and Windows 10 x64, using Microsoft IME and the steps from comment 21, but I couldn't. 
Maybe there are some steps that I'm missing.
Makoto Kato, can you please check if this bug is reproducing for you on latest Nightly and beta 58.0b10?
Thank you very much.

Comment 25

[Makoto Kato [:m_kato]]

Due to all hands, I don't have Windows PC now.  So I will verify this at next week.

Comment 26

[Makoto Kato [:m_kato]]

(In reply to Oana Botisan from comment #24)
> I tried to reproduce the bug both on Mac OS 10.11 and Windows 10 x64, using
> Microsoft IME and the steps from comment 21, but I couldn't. 
> Maybe there are some steps that I'm missing.
> Makoto Kato, can you please check if this bug is reproducing for you on
> latest Nightly and beta 58.0b10?
> Thank you very much.

I verified using 58.0b12

Comment 27

[Cornel Ionce [:noni] [Hubs QA]]

Based on the above comment.