Closed Bug 1272592 Opened 8 years ago Closed 8 years ago

[UI Events-key] Drop "MediaSelect" key value and use "LaunchMediaPlayer" instead

Categories

(Core :: DOM: Events, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla49
Tracking Status
firefox49 --- fixed

People

(Reporter: masayuki, Assigned: masayuki)

References

Details

(Keywords: dev-doc-complete, site-compat, Whiteboard: [parity-Chrome] btpp-active)

Attachments

(1 file)

"MediaSelect" key value is dropped from the latest draft because it's merged with "LaunchMediaPlayer".
https://github.com/w3c/uievents-key/commit/05ab87504c57652a3aca5d5219eec393b7c6266b
How often will the spec still change? I mean if any key value may change to something else, should we stop shipping the whole key property? (I'm not serious with that proposal, but just wondering why the spec keeps changing.)
(In reply to Olli Pettay [:smaug] from comment #1)
> How often will the spec still change?

Nobody must be not sure, unfortunately. But the spec is stepping forward, obviously.

> I mean if any key value may change to
> something else, should we stop shipping the whole key property? (I'm not
> serious with that proposal, but just wondering why the spec keeps changing.)

I don't think so. Google Chrome will support KeyboardEvent.key and KeyboardEvent.code soon (I'm not sure the version, though) with current draft.

Additionally, those attributes are more stable than .keyCode for web developers because web developers can check with a key like |if (aEvent.key == "LegacyName" || aEvent.key == "NewName")|.

And also some keys are only available with .key (i.e., .keyCode may not have specific value). They are important for supporting such key. Especially on Gaia.
Whiteboard: btpp-active
Google Chrome uses "LaunchMediaPlayer".
Whiteboard: btpp-active → [parity-Chrome] btpp-active
"MediaSelect" is merged into "LanuchMediaPlayer" in the spec since there is no difference between them and "LaunchMediaPlayer" explains the meaning clearer.

This patch removes "MediaSelect" key value and maps all keys which are mapped to "MediaSelect" to "LaunchMediaPlayer".

Review commit: https://reviewboard.mozilla.org/r/54240/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/54240/
Attachment #8754779 - Flags: review?(bugs)
Comment on attachment 8754779 [details]
MozReview Request: Bug 1272592 Merge "MediaSelect" key value into "LaunchMediaPlayer" r?smaug

https://reviewboard.mozilla.org/r/54240/#review50932

This is backwards incompatible change, but I guess fine.
Attachment #8754779 - Flags: review?(bugs) → review+
(In reply to Olli Pettay [:smaug] from comment #5)
> Comment on attachment 8754779 [details]
> MozReview Request: Bug 1272592 Merge "MediaSelect" key value into
> "LaunchMediaPlayer" r?smaug
> 
> https://reviewboard.mozilla.org/r/54240/#review50932
> 
> This is backwards incompatible change, but I guess fine.

Yeah, this should be used internally (e.g., our chrome and/or add-ons).

Even if some add-ons use this key value, they can support both key values with:

if (event.key == "MediaSelect" || event.key == "LaunchMediaPlayer") {
  // do something
}

or

switch (event.key) {
  ...
  case "MediaSelect":
  case "LaunchMediaPlayer":
    // do something
    break;
  ...
}

Anyway, these names indicate same function. That's the reason why they are merged. Additionally, "LaunchMediaPlayuer" is already mapped on other platforms. So, in most cases (even though handling these key values are rare case), nobody will be broken by this change, I guess.
This landed before some of the requisite commits did, which caused a bunch of failures. This is due to bug 1274482.

Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/9acf8466a3f5

We'll need to re-land this once the other commits land.
Flags: needinfo?(masayuki)
https://hg.mozilla.org/mozilla-central/rev/4bbdb484d7b8
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
Firefox 49 for developers has been updated. I have a question in to :masayuki about some existing content in the key event docs that I need to clarify before I update the KeyboardEvent.key and KeyboardEvent.code pages as necessary; there are some things on this page that need to be cleaned up.
This is now corrected on the KeyboardEvent.key page; I haven't looked at KeyboardEvent.code yet to see what if anything needs to be done there. That page will be rewritten soonish.
The KeyboardEvent.code page has been updated as well. That page will be totally redone soon but for now at least it's correct on this point.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: