Closed Bug 1562796 Opened 4 months ago Closed 4 months ago

IME composition does not happen directly in address field after new window creation

Categories

(Core :: User events and focus handling, defect)

x86_64
Windows 10
defect
Not set

Tracking

()

VERIFIED FIXED
mozilla69
Tracking Status
firefox-esr60 --- unaffected
firefox-esr68 --- verified
firefox67 --- unaffected
firefox68 --- verified
firefox69 --- verified

People

(Reporter: t.matsuu, Assigned: hsivonen)

References

(Regression)

Details

(Keywords: inputmethod, regression)

Attachments

(1 file)

How to reproduce:

  1. Ctrl+N
  2. Ctrl+L
  3. If Google Japanese Input is not Japanese input mode, Type "Hankaku/Zenkaku" key to enable it.
  4. Type something to input Japanese

Actual result:
Inputing Japanese characters and candidates are shown on input dialog.
After I confirm the candidate, they are shown on Address Bar.

Expected result:
Inputing Japanese characters and candidates should be shown to Address Bar.

Another scenario:
How to reproduce:

  1. Ctrl+N
  2. Click Address Bar
  3. If Google Japanese Input is not Japanese input mode, Type "Hankaku/Zenkaku" key to enable it.
  4. Type something to input Japanese

Actual result:
In most case, same as above.

In some case, no inputting Japanese characters are not shown on the Address bar though candidates are shown under the Address bar. No input dialog is shown.
After I confirm the candidate, they are not shown on the Address bar.

Expected result:
Inputting and confirmed Japanese characters should be shown to Address Bar.

This seems be to regression by bug 1549930 according to moz-regression

Component: Editor → User events and focus handling
Flags: needinfo?(hsivonen)
Regressed by: 1549930

This is broader than just the Google Japanese IME. Also applies to Microsoft IMEs and not just for Japanese.

The input dialog goes away and the composition is rendered by Gecko after any kind of focus change in the newly-created window has taken place.

Even before the fix for bug 1549930, new window creation was a known case of previously-focused BrowserParents getting popped as a side effect of new ones getting pushed before the deactivation flow could reach the push/pop code. Since the parent process (including the location bar) is represented by a null BrowserParent, maybe there's a problem with handling the case where the newly-focused thing in the new window is the location bar rather than Web content.

Summary: Japanese characters are not input directly into Address Bar → IME composition does not happen directly in address field after new window creation

This is broader than just the Google Japanese IME. Also applies to Microsoft IMEs and not just for Japanese.

Correction: The other IMEs are affected by the steps to reproduce only if the Google IME has been previously active during the lifetime of the Firefox process. Weird. However, with other IMEs, the problem can be reproduced by placing focus in address bar, raising another window and then raising the window whose focus is in the address bar.

So the problem seems to be that the code assumes that newly-created windows initially focuses a BrowserParent, which is the case I observed when I was writing the code prior to bug 1549930. (Now I'm curious why I observed that case and not the case seen here.)

The null BrowserParent, i.e. browser chrome is never pushed. Getting the stack in the state that represents chrome focus requires pops. The fix for bug 1549930 suppressed pops for BrowserParents in a window being lowered. That fix also accidentally assumes that in the window being raised, focus is in Web content so a push occurs. When the focus is in chrome, no push occurs.

We need the act of raising a window (or creating a new one) to pop everything off the stack before any Web content in the window potentially experiences its Activate.

Assignee: nobody → hsivonen
Status: NEW → ASSIGNED
Flags: needinfo?(hsivonen)

What are our options for 68 here? We already built RC1, so time is short. Should we leave things as-is and ship with this issue, or backout bug 1549930?

Flags: needinfo?(m_kato)
Flags: needinfo?(hsivonen)

(In reply to Julien Cristau [:jcristau] from comment #4)

What are our options for 68 here?

Let's see if this patch works:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=4aefd767b6af1d175ae23cfce65ec61c16ba5449

We already built RC1, so time is short. Should we leave things as-is and ship with this issue, or backout bug 1549930?

I think m_kato can judge better than I can which is worse: The location bar composition being rendered in an auxiliary window instead of the location bar in some cases (this bug) or the ATOK character palette not working (bug 1549930).

Flags: needinfo?(hsivonen)
Summary: IME composition does not happen directly in address field after new window creation → IME composition does not happen directly in address field it was
Summary: IME composition does not happen directly in address field it was → IME composition does not happen directly in address field after new window creation

(In reply to Henri Sivonen (:hsivonen) from comment #5)

(In reply to Julien Cristau [:jcristau] from comment #4)

What are our options for 68 here?

Let's see if this patch works:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=4aefd767b6af1d175ae23cfce65ec61c16ba5449

It appears to work.

(In reply to Henri Sivonen (:hsivonen) from comment #5)

(In reply to Julien Cristau [:jcristau] from comment #4)

What are our options for 68 here?

Let's see if this patch works:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=4aefd767b6af1d175ae23cfce65ec61c16ba5449

We already built RC1, so time is short. Should we leave things as-is and ship with this issue, or backout bug 1549930?

I think m_kato can judge better than I can which is worse: The location bar composition being rendered in an auxiliary window instead of the location bar in some cases (this bug) or the ATOK character palette not working (bug 1549930).

I think that it is better that this is fixed in 68 since both (this and bug 1549930) is critical. If we have to back out bug 1549930, we also should back out bug 1554843 and bug 1524242 too.

Flags: needinfo?(m_kato)
Pushed by hsivonen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1c0bc8d1aa55
Pop BrowserParents of the previous window when raising/creating a window. r=m_kato

(In reply to Makoto Kato [:m_kato] from comment #8)

(In reply to Henri Sivonen (:hsivonen) from comment #5)

(In reply to Julien Cristau [:jcristau] from comment #4)

What are our options for 68 here?

Let's see if this patch works:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=4aefd767b6af1d175ae23cfce65ec61c16ba5449

We already built RC1, so time is short. Should we leave things as-is and ship with this issue, or backout bug 1549930?

I think m_kato can judge better than I can which is worse: The location bar composition being rendered in an auxiliary window instead of the location bar in some cases (this bug) or the ATOK character palette not working (bug 1549930).

I think that it is better that this is fixed in 68 since both (this and bug 1549930) is critical. If we have to back out bug 1549930, we also should back out bug 1554843 and bug 1524242 too.

At this point, backing out bug 1524242 or related bugs is probably riskier than either uplifting the patch here or backing out bug 1549930 on its own.

Can we get this fix verified ASAP and an uplift request to release and esr68 submitted? 68 rc2 is due today or tomorrow, depending on other bugs.

Flags: qe-verify+
Status: ASSIGNED → RESOLVED
Closed: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla69
Status: RESOLVED → VERIFIED

Comment on attachment 9075343 [details]
Bug 1562796 - Pop BrowserParents of the previous window when raising/creating a window.

Beta/Release Uplift Approval Request

  • User impact if declined: When there has been no focus movement in a newly-created window or a newly-raised window has its focus in chrome, IME composition may be rendered in an auxiliary palette instead of being rendered in-place by Gecko.
  • Is this code covered by automated tests?: No
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: No
  • If yes, steps to reproduce: (I marked this as "No", since both the reporter and I already checked. However, since this is marked qe-verify+, here are the steps.)
  1. Use Windows
  2. Install the Google Japanese IME from https://www.google.co.jp/ime/
  3. Launch Firefox enough times to get rid of any "first run" special cases that focus content. You are ready for the next step when pressing ctrl-n to open a new window leaves focus in the location bar.
  4. Choose the Google Japanese IME as the input method.
  5. Put the IME into the Hiragana mode. (Can be done by mouse by clicking the menu that initially shows as its icon the letter "A" in the task bar.)
  6. Press ctrl-n.
  7. Check that the location bar has focus.
  8. Type some romaji, e.g. the qwerty keys "nihongo".

Without the fix, Japanese characters are rendered in an auxiliary palette near the top left corner of the screen. With the fix, they are rendered directly in the location bar.

  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): The mechanism of the regression from bug 1549930 as well as the fix are rather straight-forward.
  • String changes made/needed: None

ESR Uplift Approval Request

  • If this is not a sec:{high,crit} bug, please state case for ESR consideration: Bad user experience with IMEs in some cases.
  • User impact if declined: When there has been no focus movement in a newly-created window or a newly-raised window has its focus in chrome, IME composition may be rendered in an auxiliary palette instead of being rendered in-place by Gecko.
  • Fix Landed on Version: 69
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): The mechanism of the regression from bug 1549930 as well as the fix are rather straight-forward.
  • String or UUID changes made by this patch: None
Attachment #9075343 - Flags: approval-mozilla-esr68?
Attachment #9075343 - Flags: approval-mozilla-beta?

Choose the Google Japanese IME as the input method.

Furthermore, if you are checking that this fix didn't re-regress bug 1549930, be sure to switch to ATOK and relaunch Firefox before testing bug 1549930.

QA Whiteboard: [qa-triaged]
Attachment #9075343 - Flags: approval-mozilla-beta? → approval-mozilla-release?

Comment on attachment 9075343 [details]
Bug 1562796 - Pop BrowserParents of the previous window when raising/creating a window.

IME regression fix, approved for 68 rc2

Attachment #9075343 - Flags: approval-mozilla-release?
Attachment #9075343 - Flags: approval-mozilla-release+
Attachment #9075343 - Flags: approval-mozilla-esr68?
Attachment #9075343 - Flags: approval-mozilla-esr68+

Reproduced the initial issue as described in comment 13 using old Nightly build from 2019-07-01. Verified that using new builds: latest Nightly 69.0a1, 68.0 RC and esr68.0 on Windows 10, the issue does not reproduce anymore.

You need to log in before you can comment on or make changes to this bug.