991899 - Unprefix pointer lock

Status: RESOLVED FIXED

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

Comment 1

[David Bruant]

What are the conditions for unprefixing?
Are there pending spec issues?
Are there known bugs/cross-browser inconsistencies?

Comment 2

[Vincent Scheib ([e-mail])]

[Edge] and [Chrome] support pointer lock unprefixed. The [webkit prefix] in Chrome was removed. The [spec] is at Candidate Recomendation with no issues. The [next spec step] in moving the spec forward is to develop testharness tests. 

[Edge] https://dev.windows.com/en-us/microsoft-edge/platform/status/pointerlockmouselock?filter=f3f0000bf&search=pointer%20lock
[Chrome] https://www.chromestatus.com/features/6753200417800192
[webkit prefix] https://bugs.chromium.org/p/chromium/issues/detail?id=398457
[spec] https://www.w3.org/TR/pointerlock/
[next spec step] https://lists.w3.org/Archives/Public/public-webapps/2015OctDec/0116.html

Comment 3

[Xidorn Quan [:xidorn] UTC+11]

(In reply to Vincent Scheib ([e-mail]) from comment #2)
> [Edge] and [Chrome] support pointer lock unprefixed. The [webkit prefix] in
> Chrome was removed. The [spec] is at Candidate Recomendation with no issues.
> The [next spec step] in moving the spec forward is to develop testharness
> tests. 

Do you meet any compatibility issue when removing the prefixed API?

If the interface of the API was never changed after browsers implemented it, I suppose everything should be fine.

Comment 4

[Vincent Scheib ([e-mail])]

Chromium's unprefixing changed the API name only. There was no behavior change. The API has remained stable for several years now.

Comment 5

[Xidorn Quan [:xidorn] UTC+11]

Attachment: Bug 991899 part 1 - Add unprefixed API for PointerLock.

Review commit: https://reviewboard.mozilla.org/r/67314/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/67314/

Comment 6

[Xidorn Quan [:xidorn] UTC+11]

Attachment: Bug 991899 part 2 - Unprefix PointerLock tests.

Review commit: https://reviewboard.mozilla.org/r/67316/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/67316/

Comment 7

[Xidorn Quan [:xidorn] UTC+11]

Attachment: Bug 991899 part 3 - Disable prefixed PointerLock API by default.

Given that Blink has removed prefixed PointerLock API for quite a while
without receiving compatibility issue, I'd suggest we try dropping the
prefixed version directly.

We will either pref the prefixed API on if we see enough compatibility
issue, or remove the whole bunch of prefixed PointerLock API after the
unprefixed API reaches release channel without issues.

Review commit: https://reviewboard.mozilla.org/r/67318/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/67318/

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8774976 [details]
Bug 991899 part 1 - Add unprefixed API for PointerLock.

https://reviewboard.mozilla.org/r/67314/#review64466

Comment 9

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8774977 [details]
Bug 991899 part 2 - Unprefix PointerLock tests.

https://reviewboard.mozilla.org/r/67316/#review64470

Comment 10

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8774978 [details]
Bug 991899 part 3 - Disable prefixed PointerLock API by default.

https://reviewboard.mozilla.org/r/67318/#review64472

I guess we can try this.

::: dom/events/EventListenerManager.cpp:1166
(Diff revision 1)
> +      switch (aEventMessage) {
> +        case ePointerLockChange:
> +          return eMozPointerLockChange;
> +        case ePointerLockError:
> +          return eMozPointerLockError;
> +      }

don't you get some warning here about not handling all the cases? (missing default)

Comment 11

[Xidorn Quan [:xidorn] UTC+11]

https://reviewboard.mozilla.org/r/67318/#review64472

> don't you get some warning here about not handling all the cases? (missing default)

Yeah, you're right... Now I know why the webkit thing uses lots of if... I tend to just add a "default:" at the end rather than changing to use list of ifs.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/996b4f2f9963
part 1 - Add unprefixed API for PointerLock. r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/c03efdd8732f
part 2 - Unprefix PointerLock tests. r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/76ead0b044bb
part 3 - Disable prefixed PointerLock API by default. r=smaug

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/1a7af0fca900
followup - Mark some pointerlock wpt as expected-pass on CLOSED TREE

Comment 14

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/996b4f2f9963
https://hg.mozilla.org/mozilla-central/rev/c03efdd8732f
https://hg.mozilla.org/mozilla-central/rev/76ead0b044bb
https://hg.mozilla.org/mozilla-central/rev/1a7af0fca900

Comment 15

[Kohei Yoshino [:kohei]]

Posted the site compatibility doc: https://www.fxsitecompat.com/en-CA/docs/2016/prefixed-pointer-lock-api-has-been-removed/

Comment 16

[Will Bamberg [:wbamberg]]

I've updated the compat tables for this API to indicate that it's no longer prefixed from Firefox 50:
https://developer.mozilla.org/en-US/docs/Web/API/Pointer_Lock_API

Also updated Firefox 50 for developers:
https://developer.mozilla.org/en-US/Firefox/Releases/50