1593157 - Clarify what should <key key="" charcode="something"> behave

Status: NEW

(Core :: XUL, defect, P5)

Description

[Edgar Chen [:edgar]]

This bug is filed as a followup of https://phabricator.services.mozilla.com/D51241#1556900.

It is about https://searchfox.org/mozilla-central/rev/ce02064d8afc8673cef83c92896ee873bd35e7ae/dom/events/GlobalKeyListener.cpp#61-68.

In particular, if the element has key="" (empty value) and charCode="something", the old code would take the early return (because the GetAttr for "key" would return true, but valKey would be empty)

Comment 1

[Edgar Chen [:edgar]]

Comment mentions that the <key key=""> listens to no key, but it is not clear what should <key key="" charCode="something"> behave. And if you look at KeyEventHandler, it does use the value of charcode attribute if key attribute is empty string. Maybe @masayuki would recall which behavior is what we want.

Comment 2

[Edgar Chen [:edgar]]

(In reply to Edgar Chen [:edgar] from comment #1)

Comment mentions that the <key key=""> listens to no key, but it is not clear what should <key key="" charCode="something"> behave. And if you look at KeyEventHandler, it does use the value of charcode attribute if key attribute is empty string. Maybe @masayuki would recall which behavior is what we want.

More specificly, should <key key="" charCode="something"> behave the same as <key key="">?

Comment 3

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

Depending on current l10n resource data. The reason why <key key=""/> should be ignored is, optional key elements only for some languages should do nothing if applied l10n resource sets empty string (since the location's keyboard layout does not require second or third option for a command). So, I feel that it's odd such <key> elements have charCode attribute. I think that ignoring charCode attribute does not cause any problem actually. And if we meat such <key> elements, we should put a warning like when bad key combination is specified (and also here.

Comment 4

[:Gijs (out for now; he/him)]

I don't think any frontend code actually uses the charcode attribute,

https://searchfox.org/mozilla-central/search?q=charcode%3D%22

but perhaps I'm missing something? (Also no hits in comm-central)

Comment 5

[Brian Grinstead [:bgrins]]

(In reply to :Gijs (he/him) from comment #4)

I don't think any frontend code actually uses the charcode attribute,

https://searchfox.org/mozilla-central/search?q=charcode%3D%22

but perhaps I'm missing something? (Also no hits in comm-central)

I don't see anything doing setAttribute either (https://searchfox.org/mozilla-central/search?q=%22charcode%22&regexp=true&path=). Could we remove this feature?

It'd be nice to not have to worry about supporting it as we migrate away from the current <key> implementation to use a Custom Element wrapper around a new programmatic API as outlined in Bug 1550061 and "Part 2" of https://docs.google.com/document/d/1_Z2P5LeISwXt7JKuS9rKJkJg-ubWbMWUIMXPnbBrywg/edit.

Comment 6

[BugBot [:suhaib / :marco/ :calixte]]

The priority flag is not set for this bug.
:bgrins, could you have a look please?

For more information, please visit auto_nag documentation.