"ContextMenu" is defined. It's better for the context menu key on PC keyboard. We map "Menu" for it now.

We should land .key value changes in 33.

Comment on attachment 8434733 [details] [diff] [review] Patch First, let's rename what we already implemented to the newest draft's names.

Comment on attachment 8434733 [details] [diff] [review] Patch So this might break existing apps. I think we need to add a warning first, and have the at least in one release before changing the .key

This kind of changes are annoying. In a release X we use the old names, and then in some x + 1 release change to new names. How stable is the spec? Are we sure all these new names won't change?

(In reply to Olli Pettay [:smaug] from comment #4) > Comment on attachment 8434733 [details] [diff] [review] > Patch > > So this might break existing apps. What apps are you expecting? If you worry about apps on Firefox OS, we don't need to worry about non-hardware keys since Gaia's VKB API does not support D3E yet. > I think we need to add a warning first, and have the at least in one release > before changing the .key What style do you expect? I think that warning to console isn't good enough to notify of changes. (In reply to Olli Pettay [:smaug] from comment #5) > This kind of changes are annoying. In a release X we use the old names, and > then in some x + 1 > release change to new names. The value is string. Therefore, web apps can do like this: if (event.key == "OldKeyName" || event.key == "NewKeyName" || event.key == "OldIEKeyName") > How stable is the spec? Are we sure all these new names won't change? I believe that the spec is almost stable for existing key names. Although, new key names could come.

A lot of key names have been changed by feedback of me, unfortunately :-( I warned to change some key names in MDN since D3E spec starts to reorganize the key values. https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent.key#Key_values_on_Windows_%28and_char_values_of_IE%29 # green cells. So, I hope that some developers who tried to use .key are already warned by my documentation in MDN or the spec. The renaming in spec occurred some months ago. So, I think that enough term is already passed.

Asked on IRC: > [09:05] <masayuki> Yes, I have a question before starting discussion. Do you think the .key values which are already defined is enough stabe? Or is it possible to rename? > [09:06] <masayuki> Our reviewer worries *when* is the best timing to change the key names. > [09:08] <masayuki> Although, I believe that at least, existing key names are enough stable. > [09:08] <garykac> We are not planning any changes to the key names. > [09:08] <garykac> I think that the only changes that might happen would be to add new key names. > [09:09] <garykac> The key names have been stable for a while - there haven't been any bugs about renaming, only adding new ones or clarifying edge cases.

(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #6) > (In reply to Olli Pettay [:smaug] from comment #4) > > Comment on attachment 8434733 [details] [diff] [review] > > Patch > > > > So this might break existing apps. > > What apps are you expecting? If you worry about apps on Firefox OS, we don't > need to worry about non-hardware keys since Gaia's VKB API does not support > D3E yet. I mean web apps, web pages. Some page might already expect event.key to be Menu for example, but it becomes now ContextMenu. > > > I think we need to add a warning first, and have the at least in one release > > before changing the .key > > What style do you expect? I think that warning to console isn't good enough > to notify of changes. We don't have anything better. And we use console warnings for various messages about API deprecation. > > (In reply to Olli Pettay [:smaug] from comment #5) > > This kind of changes are annoying. In a release X we use the old names, and > > then in some x + 1 > > release change to new names. > > The value is string. Therefore, web apps can do like this: Can, sure. But if web app uses the old key, it will just break if it isn't updated when a new FF version with new key names are released. > > if (event.key == "OldKeyName" || > event.key == "NewKeyName" || > event.key == "OldIEKeyName") > > > How stable is the spec? Are we sure all these new names won't change? > > I believe that the spec is almost stable for existing key names. Although, > new key names could come. Adding more keys is fine. I'm just worried about changing existing names.

(In reply to Olli Pettay [:smaug] from comment #9) > (In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #6) > > (In reply to Olli Pettay [:smaug] from comment #4) > > > I think we need to add a warning first, and have the at least in one release > > > before changing the .key > > > > What style do you expect? I think that warning to console isn't good enough > > to notify of changes. > > We don't have anything better. And we use console warnings for various > messages about API deprecation. Yeah, agreed. However, only for it which might not reach to web developers, should we wait more 6 weeks? If we're in middle of a cycle, I agree with that. However, it's very bad timing... > > (In reply to Olli Pettay [:smaug] from comment #5) > > > This kind of changes are annoying. In a release X we use the old names, and > > > then in some x + 1 > > > release change to new names. > > > > The value is string. Therefore, web apps can do like this: > Can, sure. But if web app uses the old key, it will just break if it isn't > updated when a new FF version with new key names > are released. Yes. However, I don't believe that warning on console for a cycle before landing these patches makes such web applications fixed. Therefore, I don't want to wait 6 weeks for such reason. > > if (event.key == "OldKeyName" || > > event.key == "NewKeyName" || > > event.key == "OldIEKeyName") > > > > > How stable is the spec? Are we sure all these new names won't change? > > > > I believe that the spec is almost stable for existing key names. Although, > > new key names could come. > Adding more keys is fine. I'm just worried about changing existing names. Yeah, see comment 8. So, as I said in comment 7, I believe that both updating D3E spec and documenting the rename plan in MDN for a lot of months are better notification for web developers than warning on console. In other words, .key users should know the latest spec. Then, they should already include the new key name check in their apps. And also, I want to go forward our D3E implementation as soon as possible. Therefore, I want to land them as soon as possible. Even with those reasons, do you keep your mind/decision?

I doubt web devs read D3E spec. And not all read MDN (although MDN is rather popular, I think). Could we perhaps add the warnings to the current Aurora?

(In reply to Olli Pettay [:smaug] from comment #11) > I doubt web devs read D3E spec. And not all read MDN (although MDN is rather > popular, I think). I think that console warning which is suddenly starts is also not checked by every developer... > Could we perhaps add the warnings to the current Aurora? Yeah, if it's possible, I'm happy.

(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #12) > (In reply to Olli Pettay [:smaug] from comment #11) > > Could we perhaps add the warnings to the current Aurora? > > Yeah, if it's possible, I'm happy. Hmm, I'm trying to write a patch for warning on console. However, there are some problems: 1. I have no idea when we put a warning message on the console except in KeyboardEvent::Key(). 2. If we warn the message from KeyboardEvent::Key(), it should be performed only when its key value is an obsolete key name. However, this is performed every time Javascript to refer the .key attribute. 3. We need to localize the warning message in the console. So, I believe that it's difficult to uplift the patch to Aurora. #1 and #2 are also very annoying problem. For example, if following code is in web application, it will be warned 3 times when the event has obsolete key name. if (event.key == "NewName" || event.key == "OldName" || event.key == "IEName") { However, if it's comparing with non-obsoleting key names, the warning message must make web developers confused. If we would warn only once per KeyboardEvent instance, it means that the warning is shown only when the key value is referred first time. I.e., if a document touches .key attribute, add-on developers may not realize the issue in their addons, and vise versa. Additionally, this means that web developers never see the warning even if they show console always until *they* press the keys which will be obsolete. This means that except Arrow keys, most developers must not realize what key values will be obsolete. Finally, #3 means that we must need to wait 6 weeks. And also the patch would be big...

I.e., we must have no way to warn only when event.key == "ObsoleteKeyName".

Sorting out my thought: I think that it's too difficult to uplift a patch which needs l10n resource change in current tree rules. Therefore, if we would warn these changes in the console, we need to wait 6 weeks for landing the reviewed patches. Although, if we can warn the key name change effectively and absolutely, it's worthwhile to wait 6 weeks. Otherwise, I believe that it doesn't make sense to wait 6 weeks. However, I have no idea to warn this in the console only when .key is compared with obsolete key name. If we always warn this when somebody refers .key attribute, we can put it only once per event or put a lot of warnings per event which may look like warning spam. If a web app doesn't compare with obsolete key name but it's warned, this must look strange to the developers. KeyboardEvent.key is still not so stable feature of D3E. Actually, IE users old key names (some of them are different from Gecko) and Blink/WebKit have never supported. Therefore, we can guess that web apps which are using KeyboardEvent.key is very rare because of not available on Blink/WebKit. Additionally, I guess that web developers who know advanced/experimental feature like KeyboardEvent.key refer D3E spec directly because it was modified almost every week at least this year. So, I doubt that it's worthwhile to wait a cycle before these changes only for the rare developers. Finally, Garykac (a D3E editor) said that the current key names won't be changed. I.e., the defined key names of current spec are enough stable. I agree with this. IIRC, no key names are modified in the spec for this several months. I've waited for several months until D3E WG don't modify existing key names for long time and renaming request bugs are not filed. Therefore, I think we can land the patches without the warning. Although, it may not be ideal approach.

(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #13) > Finally, #3 means that we must need to wait 6 weeks. And also the patch > would be big... How so? Just add a switch-case -like thing to ::Key() implementation. And warn once per KeyboardEvent instance, and per document. We have added warnings during Aurora. I think Beta is too late. The warning message should be such that it doesn't say about use of deprecated feature, but it tells that key value "Foo" won't be supported in the future version of Firefox.

(In reply to Olli Pettay [:smaug] from comment #16) > (In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #13) > > Finally, #3 means that we must need to wait 6 weeks. And also the patch > > would be big... > How so? We need to change l10n string. And if it cannot contain variable like %s, we need to add a lot of messages. Is it available? > Just add a switch-case -like thing to ::Key() implementation. And > warn once per KeyboardEvent instance, and per document. Did you mean that an instance should warn at first time of a call in every document? How can we do that? I think that Event::mOwner->GetExtantDoc() is the third param of nsContentUtils::ReportToConsole(). And I'm guessing that it's the window of event target. Isn't it? Therefore, I have no idea how to warn on every document only once. > We have added warnings during Aurora. I think Beta is too late. That's good news to me! > The warning message should be such that it doesn't say about use of > deprecated feature, but > it tells that key value "Foo" won't be supported in the future version of > Firefox. I'm assuming that you want a message like: "%s is obsolete, it will be renamed to new name which is defined in DOM Level 3 KeyboardEvent key Values" The %s is the key name stored by the key event. But is it ok? Until such developers press the keys which will be renamed, they never see the warning. Even if they check the console.

(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #17) > We need to change l10n string. And if it cannot contain variable like %s, we > need to add a lot of messages. Is it available? %S should work. > > > Just add a switch-case -like thing to ::Key() implementation. And > > warn once per KeyboardEvent instance, and per document. > > Did you mean that an instance should warn at first time of a call in every > document? How can we do that? I think that Event::mOwner->GetExtantDoc() is > the third param of nsContentUtils::ReportToConsole(). And I'm guessing that > it's the window of event target. Isn't it? Therefore, I have no idea how to > warn on every document only once. See nsIDocument::WarnOnceAbout > I'm assuming that you want a message like: > > "%s is obsolete, it will be renamed to new name which is defined in DOM > Level 3 KeyboardEvent key Values" Something like that. Perhaps KeyboardEvent.key value %s is obsolete.... > But is it ok? Until such developers press the keys which will be renamed, > they never see the warning. Even if they check the console. Well, I don't know how else we could warn about rather significant API change.

(In reply to Olli Pettay [:smaug] from comment #18) > (In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #17) > > > Just add a switch-case -like thing to ::Key() implementation. And > > > warn once per KeyboardEvent instance, and per document. > > > > Did you mean that an instance should warn at first time of a call in every > > document? How can we do that? I think that Event::mOwner->GetExtantDoc() is > > the third param of nsContentUtils::ReportToConsole(). And I'm guessing that > > it's the window of event target. Isn't it? Therefore, I have no idea how to > > warn on every document only once. > > See nsIDocument::WarnOnceAbout Thanks. Looks like it's useful. I'll try to write warning patch in new bug.