Gecko doesn't report the correct IME action when in an HTML form field

RESOLVED FIXED in Firefox 68

Status

defect
P3
normal
RESOLVED FIXED
Last year
5 months ago

People

(Reporter: imanol, Assigned: m_kato)

Tracking

Trunk
mozilla68
Unspecified
Android

Firefox Tracking Flags

(firefox-esr52 wontfix, firefox-esr60 wontfix, firefox61 wontfix, firefox62 wontfix, firefox63 wontfix, firefox64 wontfix, firefox65 wontfix, firefox66 wontfix, firefox67 wontfix, firefox68 fixed)

Details

(Whiteboard: [geckoview:fenix:p3])

Attachments

(5 attachments)

Chrome correctly reports IME action as IME_ACTION_NEXT if there are more fields but Gecko reports IME_ACTION_DONE.

Example to reproduce it: https://www.w3schools.com/htmL/html_forms.asp

Info from m_kato: Hmm, https://cs.chromium.org/chromium/src/content/public/android/java/src/org/chromium/content/browser/input/ImeUtils.java?q=IME_ACTION_NEXT&sq=package:chromium&g=0&l=146 is chrome implementation.  If there is other input element that is focusable, we should add it for IME's hint.
Jim says P3 and this more of a platform issue than GeckoView.
Assignee: nobody → nchen
Priority: -- → P3
Unless something regressed since I last looked at this, for us the fact that a form has a default submit element (which I think is the case for those w3schools examples) takes precedence in determining the IME hint to use:
https://dxr.mozilla.org/mozilla-central/rev/085cdfb90903d4985f0de1dc7786522d9fb45596/dom/events/IMEStateManager.cpp#1340
(In reply to Jan Henning [:JanH] from comment #2)
> [...] for us the fact that
> a form has a default submit element (which I think is the case for those
> w3schools examples) takes precedence in determining the IME hint to use:

Which might come from mimicking Desktop: There, pressing Enter in any of the text input fields will submit the form, and with mobile IMEs often the only way to press Enter is when receiving an action hint of IME_ACTION_DONE.
Any idea when this is going to be addressed?
Makoto, do you have ideas on this?
Flags: needinfo?(m_kato)
We may need to set "next" to HTMLInputInputMode when calling SetInputContext from IMEStateManager.  So IMEStateManager should set "next" to aInputContext.mHTMLInputInputMode even if content has submit button.

https://searchfox.org/mozilla-central/rev/39cb1e96cf97713c444c5a0404d4f84627aee85d/dom/events/IMEStateManager.cpp#1332 doesn't consider this situation.
Flags: needinfo?(m_kato)
jchen, are you working this?  Since this bug is IMEStateManager in dom/event, I can take this.
Flags: needinfo?(nchen)
Sounds good. Thanks!
Assignee: nchen → m_kato
Flags: needinfo?(nchen)
nsFocusManager doesn't export a method that gets next focusable content....
Since event time isn't set when translating ENTER key to TAB key, comparing
event time hits an assertion.  So event time should be set.
Actually, there is no public method to get next element/content by tabIndex or
TAB key.  I would like to use GetNextTabbableContent from IMEStateManager.

Depends on D12883
Currently, IMEStateManager::SetIMEState sets hint to the following logic.

- If there is no submit button into form element, set "next"
- If there is submit button, set "search" or "go"
- If there isn't into form element, no hint.

But Chrome sets "next" hint when next focusable element is input that is text
control. So even if there is submit button into form element, we should set
"next" to hint when next focusable element is input that is text controll.

Also, there is no way to add unit test since hint isn't exposed to script.
Do you have any idea for test?

Depends on D12884
Attachment #9027473 - Attachment description: Bug 1474902 - Part 2. Make nsFocusManager::GetNextTabbableContent public → Bug 1474902 - Part 2. Make nsFocusManager::DetermineElementToMoveFocus public. r?NeilDeakin
Product: Firefox for Android → GeckoView
See Also: → 717120
Duplicate of this bug: 720475
See Also: 717120
Duplicate of this bug: 717120

To make setting action hint simple, I would like to move it to static function.

Will we want to uplift this IME fix to GV 67 Beta for Fenix MVP? Since this is not a recent regression, maybe we don't need to fix it in Fenix MVP.

Whiteboard: [geckoview:fenix:p3]

(In reply to Chris Peterson [:cpeterson] from comment #18)

Will we want to uplift this IME fix to GV 67 Beta for Fenix MVP? Since this is not a recent regression, maybe we don't need to fix it in Fenix MVP.

This is not regression. This is an improvement for software keyboard. So it is unnecessary to uplift to beta.

Attachment #9027474 - Attachment description: Bug 1474902 - Part 3. Set hint to next when next focusable element is input element that is text controll. → Bug 1474902 - Part 4. Set hint to next when next focusable element is input element that is text control.

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

This is not regression. This is an improvement for software keyboard. So it is unnecessary to uplift to beta.

Thanks. In that case, I will set status flag 67=wontfix.

Pushed by m_kato@ga2.so-net.ne.jp:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c54533bd0e04
Part 1. Set event time when translating to TAB key r=esawin
https://hg.mozilla.org/integration/mozilla-inbound/rev/05cee469e93e
Part 2. Make nsFocusManager::DetermineElementToMoveFocus public. r=NeilDeakin
https://hg.mozilla.org/integration/mozilla-inbound/rev/1848a0f1a982
Part 3. Move setting action hint to static function. r=masayuki
https://hg.mozilla.org/integration/mozilla-inbound/rev/f190032bbb2e
Part 4. Set hint to next when next focusable element is input element that is text control. r=masayuki
https://hg.mozilla.org/integration/mozilla-inbound/rev/98305e6fa0d6
Part 5. Add mochitest. r=masayuki
You need to log in before you can comment on or make changes to this bug.