1490661 - Support enterkeyhint and deprecate mozactionhint

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect, P2)

Description

[dtapuska]

User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/69.0.3497.81 Safari/537.36

Steps to reproduce:

enterkeylabel is a standardization of mozactionhint (with previous addition)

https://github.com/whatwg/html/pull/3538


Actual results:

Not supported


Expected results:

Support the new attribute and pass the WPT tests

Comment 1

[dtapuska]

err.

Comment 2

[dtapuska]

Clearly I wasn't awake when I submitted this. It should reference enterkeyhint

Comment 3

[dtapuska]

Example: http://output.jsbin.com/mukira

Comment 4

[Boris Zbarsky [:bzbarsky]]

So if I understand correctly, this requires two changes:

1)  Adding an IDL attribute with reflection for the new attributes.

2)  Adding UI functionality to actually change what the user is shown based on those attributes.

Is that correct?

Should UAs avoid doing #1 until #2 is done, to allow feature-detection?   What should happen on platforms where #2 is not really possible (because there is no virtual keyboard)?

Comment 5

[dtapuska]

You need to do 1).

And for 2 you should already have the support on Android (except for the previous value but should be trivial to add).

This is only used as a hint so feature detection I don't think is necessary. Chrome only has control of this on Android but we do read and reflect the attribute on all platforms.

Comment 6

[Gyula Palko[:gpalko]]

Setting the component in order to involve the development team in reviewing this issue.

Comment 7

[Boris Zbarsky [:bzbarsky]]

That's the wrong component, as well as the wrong product, for what needs to be done here.

Comment 8

[Boris Zbarsky [:bzbarsky]]

So what needs to happen is:

1)  Sending an intent to implement (and ship?)
2)  Adding https://html.spec.whatwg.org/#attr-enterkeyhint ... on which elements?  Spec has it for all HTML elements, but we don't generally support this stuff for non-form-controls.   I still have open questions around feature-detection here and whether sites will end up switching on whether the attribute is present on various types of elements.   In particular, it's not clear to me that we want to support the reflection on elements on which we don't use the attribute.
3)  In IMEStateManager::SetIMEState (in dom/events/IMEStateManager.cpp) we want to consider this attribute in the case where it currently does GetAttr(nsGkAtoms::moz_action_hint).  Looks like that's only for input and textarea, so we may want to limit exposure of the IDL reflection to those elements until that changes...

Comment 9

[Makoto Kato [:m_kato]]

Also, this has unclear issue for event model. https://github.com/w3c/uievents/issues/253

Comment 10

[Cameron McCormack (:heycam)]

I see people promoting this now that Safari has shipped: https://twitter.com/stefanjudis/status/1249958064041734144

Comment 11

[Makoto Kato [:m_kato]]

Yoshino-san, although you set site-compat keyword, do you know whether any major site already uses this attribute?

To ship this on us, we have to clear ui event issue per comment #9. Although I ask it to dtapuska, no response.

Comment 12

[dtapuska]

Sorry I missed that. We always fire enter events if the attribute is specified. The specification could likely be cleaned up.

Comment 13

[Makoto Kato [:m_kato]]

(In reply to dtapuska from comment #12)

Sorry I missed that. We always fire enter events if the attribute is specified. The specification could likely be cleaned up.

Thank you. I already have a prototype for this implementation, so I am considering this for GeckoView.

Comment 14

[Kohei Yoshino]

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

Yoshino-san, although you set site-compat keyword, do you know whether any major site already uses this attribute?

I don’t know any specific case but will post a site compatibility note anyway for the deprecation.

Comment 15

[Makoto Kato [:m_kato]]

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

Comment 16

[Makoto Kato [:m_kato]]

Attachment: Bug 1490661 - Part 1. Support HTML.enterKeyHint in Nighly. r=#webidl

On mobile platform, "ENTER" key on software keyboard key can sometimes
change to "Next" and etc. Although Firefox OS/Firefox Android have
mozactionhint attribute for its behaviour, This is standardized version
of it.

Chrome and Safari already support it now, so I would like to support this
on Firefox/GeckoView Nightly.

Comment 17

[Makoto Kato [:m_kato]]

Attachment: Bug 1490661 - Part 2. Add enterkeyhint attribute to htmlparser. r=hsivonen

Bug 1618295 isn't merged into https://github.com/validator/htmlparser, I copy
it from /parser/html/javasrc/AttributeName.java

Depends on D79641

Comment 18

[Makoto Kato [:m_kato]]

Attachment: Bug 1490661 - Part 3. InputContext should reference enterkeyhint attribute for action hint. r=masayuki

Set enterkeyhint to InputContext.mActionHint. Although it is used by
moz_action attribute, enterkeyhint is standardized version of this.

New logic is the following.

1. Read enterkeyhint that is from editing host
2. Read moz_action on <input> element if no enterkeyhint
3. If both is nothing, we infer this value from the <form>.

Depends on D79642

Comment 19

[Makoto Kato [:m_kato]]

Attachment: Bug 1490661 - Part 4. Split next action hint with next and maybenext. r=masayuki

Current implementation of Blink and WebKit is that enter key is dispatched
even if enterkeyhint is next. If no enterkeyhint, Gecko and Blink infer
this type from <form> and <input> element.

If this key is set as "next" by inference, Blink doesn't dispatch key event
then sets focus to next element, and Gecko dispatches Tab key to set focus
to next element.

So if action hint is "next" by inference, we would like to change to another
type "maybenext".

Depends on D79644

Comment 20

[Makoto Kato [:m_kato]]

Attachment: Bug 1490661 - Part 5. Implement current spec of enterkeyhint on Android backend. r=#geckoview-reviewers

When using enterkeyhint attribute, key event must be be fired as enter key.
So by previous changeset, I change action type to maybenext if inferring
action type without enterkeyhint.

Also, enterkeyhint can have additional values (enter and previous), so Android
back end should support it.

Depends on D79645

Comment 21

[Makoto Kato [:m_kato]]

Attachment: Bug 1490661 - Part 6. Add geckoview-junit test for enterkeyhint. r=#geckoview-reviewers

Depends on D79646

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7c732d35b397
Part 1. Support HTML.enterKeyHint in Nighly. r=webidl,smaug,hsivonen
https://hg.mozilla.org/integration/autoland/rev/9b7202e779eb
Part 2. Add enterkeyhint attribute to htmlparser. r=hsivonen
https://hg.mozilla.org/integration/autoland/rev/3605a382f0bf
Part 3. InputContext should reference enterkeyhint attribute for action hint. r=masayuki
https://hg.mozilla.org/integration/autoland/rev/bd285002fe0c
Part 4. Split next action hint with next and maybenext. r=masayuki
https://hg.mozilla.org/integration/autoland/rev/e3090704a024
Part 5. Implement current spec of enterkeyhint on Android backend. r=geckoview-reviewers,agi
https://hg.mozilla.org/integration/autoland/rev/2e38bb7fa471
Part 6. Add geckoview-junit test for enterkeyhint. r=geckoview-reviewers,agi

Comment 23

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/7c732d35b397
https://hg.mozilla.org/mozilla-central/rev/9b7202e779eb
https://hg.mozilla.org/mozilla-central/rev/3605a382f0bf
https://hg.mozilla.org/mozilla-central/rev/bd285002fe0c
https://hg.mozilla.org/mozilla-central/rev/e3090704a024
https://hg.mozilla.org/mozilla-central/rev/2e38bb7fa471

Comment 24

[Henri Sivonen (:hsivonen)]

https://github.com/validator/htmlparser/commit/5a27aaec3e0f9e711fc5f8947ac51d24845dd6cd