Closed
Bug 851547
Opened 12 years ago
Closed 11 years ago
Make Gamepad API preffable
Categories
(Core :: DOM: Events, defect)
Core
DOM: Events
Tracking
()
RESOLVED
FIXED
mozilla24
People
(Reporter: ted, Assigned: ted)
References
Details
Attachments
(1 file, 2 obsolete files)
(deleted),
patch
|
smaug
:
review+
|
Details | Diff | Splinter Review |
We should have a pref that controls enabling the Gamepad API so that we can make it off-by-default in releases until the API stabilizes. Right now we can disable it in configure if we need to, so it's not the end of the world, but it would be nicer to have a pref so users can flip it on for testing (it's also less invasive and less likely to break the build).
Assignee | ||
Comment 1•12 years ago
|
||
This seems to work in my testing. When I get around to exposing navigator.getGamepads() that will have to be pref-controlled as well, but currently we only have the events.
I had to refactor the tests a bit so they'd still work when the pref was disabled by default. While I was there I removed test_gamepad_basic.html since it wasn't testing anything useful (it was a leftover from very early testing).
Also I realized that GamepadService wasn't implementing QI for nsIObserver, oops. (I hit some assertions while testing this patch.)
I have the pref configured in all.js so that the API will be disabled for release builds now, since the spec and implementation aren't stable yet.
Attachment #730199 -
Flags: review?(bugs)
Assignee | ||
Updated•12 years ago
|
Assignee: nobody → ted
Comment 2•12 years ago
|
||
Comment on attachment 730199 [details] [diff] [review]
Make Gamepad API preffable
This doesn't hide the events from the global scope.
Attachment #730199 -
Flags: review?(bugs) → review-
Comment 3•12 years ago
|
||
Perhaps we should disable the API for FF23 release using the configure, and once we have
webidl events, update the patch here to hide events too.
Assignee | ||
Comment 4•12 years ago
|
||
Ok. Would you rather I waited on WebIDL events to land this then, so we can do it all properly?
Comment 5•12 years ago
|
||
Well, this pref'ing should probably wait for Webidl events, but
could we just disable the configure option once the gamepad API is about to merge to beta?
Assignee | ||
Comment 6•12 years ago
|
||
Yes, that was my intent if I couldn't get an acceptable way to pref it off. If WebIDL events aren't going to make it in time then we'll just do that.
Updated•12 years ago
|
tracking-firefox22:
--- → +
Comment 7•12 years ago
|
||
I'm tracking this assuming this is where we'll also perform the disable for Beta.
Assignee | ||
Comment 8•12 years ago
|
||
Since this isn't going to make it in time for FF22 (per comment 3), should we just file a new bug to disable it at build time for beta?
Assignee | ||
Comment 9•12 years ago
|
||
bz pointed out that I could just set dom.gamepad.enabled to true in the Mochitest prefs file and skip all this pushPrefEnv stuff in the tests.
Comment 10•12 years ago
|
||
do you want to just morph this bug to be where we track disabling for 22 then?
Assignee | ||
Comment 11•12 years ago
|
||
I already have a patch here, I filed bug 857797 for that.
tracking-firefox22:
+ → ---
Whiteboard: [games:p1]
Whiteboard: [games:p1]
Blocks: 878828
Assignee | ||
Comment 12•11 years ago
|
||
I tweaked this patch a little bit to make it simpler. It no longer supports dynamically changing the pref, you have to restart to make it take effect. I also got rid of the test changes and simply force the pref on in the test harness.
Assignee | ||
Updated•11 years ago
|
Attachment #730199 -
Attachment is obsolete: true
Assignee | ||
Comment 13•11 years ago
|
||
Now that smaug added webidl interfaces for the Gamepad events in bug 847611, I can pref-disable them here. Manual testing shows this works fine--all the Gamepad* interfaces are not visible to content when the pref is disabled.
Attachment #764739 -
Flags: review?(bugs)
Assignee | ||
Updated•11 years ago
|
Attachment #763708 -
Attachment is obsolete: true
Comment 14•11 years ago
|
||
Comment on attachment 764739 [details] [diff] [review]
Make Gamepad API preffable
The nsIObserver stuff aren't really related, but ok.
I don't understand why you remove test_gamepad_basic.html
It is perhaps rather useless test though
Attachment #764739 -
Flags: review?(bugs) → review+
Assignee | ||
Comment 15•11 years ago
|
||
The nsIObserver change is not critical, it's leftover from when I had a pref observer in the first revision of the patch. It is just a correctness fix.
The test removal is also incidental, sorry, that is also leftover. The test isn't particularly useful though, so removing it is harmless.
Thanks for the review!
Assignee | ||
Comment 16•11 years ago
|
||
Comment 17•11 years ago
|
||
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
You need to log in
before you can comment on or make changes to this bug.
Description
•