Difference between revisions of "Implementing the Mouse Lock API in Firefox"
NorthWind87 (talk | contribs) (→Tests: Removed userAgentIsActive from requirements covered listing) |
(→Review Issues) |
||
(58 intermediate revisions by 6 users not shown) | |||
Line 116: | Line 116: | ||
* <s>Space, bracing, tabs, etc style nits</s> humph | * <s>Space, bracing, tabs, etc style nits</s> humph | ||
* <s>clientX, clientY, screenX, screenY values should follow spec as per [https://bugzilla.mozilla.org/show_bug.cgi?id=633602#c55 scheib's comment]</s> diogogmt | * <s>clientX, clientY, screenX, screenY values should follow spec as per [https://bugzilla.mozilla.org/show_bug.cgi?id=633602#c55 scheib's comment]</s> diogogmt | ||
− | * Confirm |aEvent->lastRefPoint = nsIntPoint(bounds.width/2, bounds.height/2);| that / 2 is right | + | * <s>Confirm |aEvent->lastRefPoint = nsIntPoint(bounds.width/2, bounds.height/2);| that / 2 is right</s> diogogmt |
− | * "nsCOMPtr<nsIContent> mMouseLockedElement; - Shouldn't this be a static member variable? Or how does the patch handle cases when the document which has iframe and mouse moves over those iframes." Probably wants a test for this case, too. | + | * <s>"nsCOMPtr<nsIContent> mMouseLockedElement; - Shouldn't this be a static member variable? Or how does the patch handle cases when the document which has iframe and mouse moves over those iframes." Probably wants a test for this case, too.</s> diogogmt |
− | * "nsDOMMouseLockable::ShouldLock...QI to nsINode and check IsInDoc()" diogogmt | + | * <s>"nsDOMMouseLockable::ShouldLock...QI to nsINode and check IsInDoc()"</s> diogogmt |
− | * "nsRefPtr<nsMouseLockableRequest> request = new nsMouseLockableRequest(aSuccessCallback, aFailureCallback); -- You should store aTarget (QI'ed to nsINode or nsIContent or Element or nsIDOMEventTarget) so that when calling callback you push Cx to stack; nsCxPusher pusher: // defined in nsContentUtils.h NS_ENSURE_STATE(pusher.Push(target));" | + | * <s>"nsRefPtr<nsMouseLockableRequest> request = new nsMouseLockableRequest(aSuccessCallback, aFailureCallback); -- You should store aTarget (QI'ed to nsINode or nsIContent or Element or nsIDOMEventTarget) so that when calling callback you push Cx to stack; nsCxPusher pusher: // defined in nsContentUtils.h NS_ENSURE_STATE(pusher.Push(target));"</s> - humph |
* <s>"nsDOMMouseLockable looks like it should be cycle collectable"</s> humph | * <s>"nsDOMMouseLockable looks like it should be cycle collectable"</s> humph | ||
− | * Break .pointer out of Navigator, and add nsIDOMMozNavigatorPointer (not sure if this name is best) as a new interface to Navigator, similar to nsIDOMMozNavigatorBattery. - | + | * <s>Break .pointer out of Navigator, and add nsIDOMMozNavigatorPointer (not sure if this name is best) as a new interface to Navigator, similar to nsIDOMMozNavigatorBattery.</s> - humph, diogogmt |
− | * Rename for moz* prefix. The API should be probably prefixed. So, navigator.mozPointer and all the interfaces should start with nsIDOMMoz. Similar to what webkit* is doing: | + | * <s>Rename for moz* prefix. The API should be probably prefixed. So, navigator.mozPointer and all the interfaces should start with nsIDOMMoz. Similar to what webkit* is doing:</s> - humph |
− | ** navigator.webkitPointer.unlock(); | + | ** <s>navigator.webkitPointer.unlock();</s> |
− | ** navigator.webkitPointer.lock(); | + | ** <s>navigator.webkitPointer.lock();</s> |
− | ** navigator.webkitPointer.isLocked(); | + | ** <s>navigator.webkitPointer.isLocked();</s> |
− | ** document.body.addEventListener("webkitpointerlocklost", ... | + | ** <s>document.body.addEventListener("webkitpointerlocklost", ...</s> |
− | ** mouseMoveEvent.webkitMovementX | + | ** <s>mouseMoveEvent.webkitMovementX</s> |
− | ** mouseMoveEvent.webkitMovementY | + | ** <s>mouseMoveEvent.webkitMovementY</s> |
* Follow discussion unfolding here for other changes: http://lists.w3.org/Archives/Public/public-webapps/2011OctDec/1558.html | * Follow discussion unfolding here for other changes: http://lists.w3.org/Archives/Public/public-webapps/2011OctDec/1558.html | ||
* Finish tests, updated to changes above. | * Finish tests, updated to changes above. | ||
+ | * <s>Update patch to trunk</s> - humph | ||
+ | * Figure out why DOMMouseScroll events aren't being retargeted to the locked element | ||
+ | |||
+ | * Look into roc's suggestion from bug 722449 comment 8, namely, using getBoundingClientRect vs. GetPrimaryFrame() | ||
+ | |||
+ | * Maybe use nsStubMutationObserver instead of nsIMutationObserver? | ||
+ | |||
+ | * Extensions to the Document Interface. Add pointerLockElement attribute and exitPointerLock method - diogogmt | ||
+ | |||
+ | * Update Element interface to have requestPointerLock - diogogmt | ||
+ | |||
+ | * <s>Dispatch pointerlockchange or pointerlockerror events instead of firing a callback when pointer gets locked</s> - diogogmt | ||
+ | |||
+ | * Update mochitests to use new pointerlock API | ||
+ | |||
+ | * Add expect number of tests to mochitests | ||
+ | * Update patch to trunk | ||
===Tests=== | ===Tests=== | ||
Line 330: | Line 347: | ||
One or more tests have to deal with switching the lock between multiple elements, and the [[Changes to Fullscreen Unlock|spec is changing]] on that front. | One or more tests have to deal with switching the lock between multiple elements, and the [[Changes to Fullscreen Unlock|spec is changing]] on that front. | ||
− | ====Changes To Be Applied To Submitted Tests==== | + | ====New Test Reviews [Updated 1/19/2012]==== |
+ | This is a new list of test. The following tests will need to confirm to the new prefixes in accordance to Patchv3 of [https://bugzilla.mozilla.org/show_bug.cgi?id=633602 Bug 633602]. Please add your name beside each test(s) you are assessing/reviewing/fixing/peer reviewing. | ||
+ | Once the cause for rejection is clear, indicate it by placing an X in any of the columns and optionally filling out the comments column. Indicate the action taken to resolve the issue in the action taken column. Example actions include: | ||
+ | # '''DELETED''' | ||
+ | # '''FIXED''' | ||
+ | |||
+ | Other actions are also acceptable where appropriate. | ||
+ | Once fixed, please pull request to rhung's [https://github.com/rhung/mozilla-central/tree/mouselock-tests/dom/tests/mochitest/mouselockrhung's mouselock-tests] branch and indicate that you've done so in the pull requested column. Ensure that you add '''[POST-SUBMIT]''' to the name of your pull request. | ||
+ | |||
+ | {| border=1 style="border: 1px solid darkgray;" | ||
+ | | Num | ||
+ | | Name | ||
+ | | Primary Reviewer | ||
+ | | Timing issues? | ||
+ | | Needs Harness? | ||
+ | | Needs focus? | ||
+ | | Formatting issues? | ||
+ | | Other? (Indicate in comments) | ||
+ | | Peer Reviewers | ||
+ | | Peer reviewed? | ||
+ | | Action Taken | ||
+ | | Pull requested? | ||
+ | | bgcolor="red" style="text-align:center;" | Updated with Patchv3 | ||
+ | | Comments | ||
+ | |- | ||
+ | | colspan=14 bgcolor="yellow" style="text-align:center;" | '''Ensure you have actually tested your test before sending a pull request''' | ||
+ | |- | ||
+ | | 1 | ||
+ | | test_FullScreenHarness.html | ||
+ | | Anachid | ||
+ | | X (no action) | ||
+ | | X (no action) | ||
+ | | X (no action) | ||
+ | | X (no action) | ||
+ | | - | ||
+ | | | ||
+ | | | ||
+ | | '''FIXED''' | ||
+ | | | ||
+ | | X | ||
+ | | | ||
+ | |- | ||
+ | | 2 | ||
+ | | file_DOMtree.html | ||
+ | | Anachid | ||
+ | | X (fixed) | ||
+ | | X (fixed) | ||
+ | | X (fixed) | ||
+ | | X (no action) | ||
+ | | - | ||
+ | | | ||
+ | | | ||
+ | | '''FIXED''' | ||
+ | | | ||
+ | | X | ||
+ | | | ||
+ | |- | ||
+ | | 3 | ||
+ | | file_nestedFullScreen.html | ||
+ | | Anachid | ||
+ | | X (fixed) | ||
+ | | X (fixed) | ||
+ | | X (fixed) | ||
+ | | X (no action) | ||
+ | | - | ||
+ | | | ||
+ | | | ||
+ | | '''FIXED''' | ||
+ | | | ||
+ | | X | ||
+ | | | ||
+ | |- | ||
+ | | 4 | ||
+ | | file_differentDOM.html | ||
+ | | Anachid | ||
+ | | X (no action) | ||
+ | | X (no action) | ||
+ | | X (no action) | ||
+ | | X (no action) | ||
+ | | - | ||
+ | | | ||
+ | | | ||
+ | | '''FIXED''' | ||
+ | | | ||
+ | | X | ||
+ | | | ||
+ | |- | ||
+ | | 5 | ||
+ | | file_fullscreen.html | ||
+ | | Anachid | ||
+ | | X | ||
+ | | X | ||
+ | | X | ||
+ | | X | ||
+ | | - | ||
+ | | | ||
+ | | | ||
+ | | '''FIXED''' | ||
+ | | | ||
+ | | X | ||
+ | | Test needs to completely re-done. Cases like <br> | ||
+ | * ok(false, "Mouse locked without fullscreen mode"); will always fail! | ||
+ | * Flow of the test will causes scope problems. | ||
+ | * Currently fixing. | ||
+ | |- | ||
+ | | 6 | ||
+ | | file_doubleLockCallBack.html | ||
+ | | Anachid | ||
+ | | X | ||
+ | | X | ||
+ | | X | ||
+ | | X | ||
+ | | - | ||
+ | | | ||
+ | | | ||
+ | | '''FIXED''' | ||
+ | | | ||
+ | | X | ||
+ | | | ||
+ | |- | ||
+ | | 7 | ||
+ | | file_unlockOnUnload.html | ||
+ | | Anachid | ||
+ | | X | ||
+ | | X | ||
+ | | X | ||
+ | | X | ||
+ | | - | ||
+ | | | ||
+ | | | ||
+ | | '''FIXED''' | ||
+ | | | ||
+ | | X | ||
+ | | | ||
+ | |- | ||
+ | | 8 | ||
+ | | file_exitMouseLockOnLoseFocus.html | ||
+ | | Anachid | ||
+ | | X | ||
+ | | X | ||
+ | | X | ||
+ | | X | ||
+ | | - | ||
+ | | | ||
+ | | | ||
+ | | '''FIXED''' | ||
+ | | | ||
+ | | X | ||
+ | | | ||
+ | |- | ||
+ | | 9 | ||
+ | | test_mousePos.html | ||
+ | | rhung | ||
+ | | | ||
+ | | | ||
+ | | | ||
+ | | | ||
+ | | Doesn't seem like it can be done via mochitest | ||
+ | | | ||
+ | | | ||
+ | | '''REMOVED''' | ||
+ | | | ||
+ | | | ||
+ | | As mentioned in the previous comments, synthesizeMouse cannot be moved beyond the browser. It might need to be a litmus test. | ||
+ | |- | ||
+ | | 10 | ||
+ | | file_mouselocklost.html | ||
+ | | Anachid | ||
+ | | X | ||
+ | | X | ||
+ | | X | ||
+ | | X | ||
+ | | - | ||
+ | | | ||
+ | | | ||
+ | | '''FIXED''' | ||
+ | | | ||
+ | | X | ||
+ | | Removed some test that were repeating. Otherwise, it works | ||
+ | |- | ||
+ | | 11 | ||
+ | | file_movementXY.html | ||
+ | | rhung | ||
+ | | X | ||
+ | | X | ||
+ | | X | ||
+ | | X | ||
+ | | - | ||
+ | | | ||
+ | | | ||
+ | | '''Fixing''' | ||
+ | | | ||
+ | | X | ||
+ | | | ||
+ | |- | ||
+ | | 12 | ||
+ | | file_targetOutOfFocus.html | ||
+ | | rhung | ||
+ | | X | ||
+ | | X | ||
+ | | X | ||
+ | | X (fixed) | ||
+ | | - | ||
+ | | | ||
+ | | | ||
+ | | '''FIXED''' | ||
+ | | | ||
+ | | X | ||
+ | | Minor changes, mostly styling. | ||
+ | |- | ||
+ | | 13 | ||
+ | | test_mouseLockable.html | ||
+ | | rhung | ||
+ | | X | ||
+ | | X | ||
+ | | X | ||
+ | | X | ||
+ | | - | ||
+ | | | ||
+ | | | ||
+ | | '''FIXED & RENAMED''' | ||
+ | | | ||
+ | | X | ||
+ | | Updated to test_MozPointerLock.html | ||
+ | |- | ||
+ | | 14 | ||
+ | | file_MouseEvents.html | ||
+ | | rhung | ||
+ | | X | ||
+ | | X | ||
+ | | X | ||
+ | | X | ||
+ | | Leak issue might be platform specific. | ||
+ | | | ||
+ | | | ||
+ | | '''FIXED?''' | ||
+ | | | ||
+ | | X | ||
+ | | Works without chance of hanging or random success and failures. | ||
+ | |- | ||
+ | | 15 | ||
+ | | file_limitlessScroll.html | ||
+ | | rhung | ||
+ | | | ||
+ | | | ||
+ | | | ||
+ | | | ||
+ | | | ||
+ | | | ||
+ | | | ||
+ | | '''MERGED WITH file_movementXY''' | ||
+ | | | ||
+ | | | ||
+ | | | ||
+ | |- | ||
+ | | 16 | ||
+ | | file_defaultUnlock.html | ||
+ | | rhung | ||
+ | | | ||
+ | | | ||
+ | | | ||
+ | | | ||
+ | | | ||
+ | | | ||
+ | | | ||
+ | | '''FIXED''' | ||
+ | | | ||
+ | | X | ||
+ | | | ||
+ | |- | ||
+ | | 17 | ||
+ | | file_userPref.html | ||
+ | | rhung | ||
+ | | | ||
+ | | | ||
+ | | | ||
+ | | | ||
+ | | | ||
+ | | | ||
+ | | | ||
+ | | '''FIXED''' | ||
+ | | | ||
+ | | X | ||
+ | | | ||
+ | |- | ||
+ | | 18 | ||
+ | | file_constantXY.html | ||
+ | | rhung | ||
+ | | | ||
+ | | | ||
+ | | | ||
+ | | | ||
+ | | | ||
+ | | | ||
+ | | | ||
+ | | '''FIXING''' | ||
+ | | | ||
+ | | X | ||
+ | | Needs proper error messages, perhaps formatting issues. Test runs fine though. | ||
+ | |- | ||
+ | |} | ||
+ | |||
+ | |||
+ | New Tests (not sure if it will work on mochitest, and the test will possibly fail as dave is working on this patch) | ||
+ | # When there is a parent element with multiple child element, have a test to check that all mouse events fired inside the child element are suppressed and is captured by the parent element. (this may also include the scenario where there is an iframe in the parent element) | ||
+ | # A Test to check that repeated move of the mouse to a certain direction that will exceed the screen size does not break the mouselock (this may conflict with file_movementXY.html and we may need to add more test cases) | ||
+ | |||
+ | ====Changes To Be Applied To Submitted Tests [Deprecated]==== | ||
The following table is used to maintain a small note on the task for each test. It will include note from the review by Mozilla and issue we overlooked. | The following table is used to maintain a small note on the task for each test. It will include note from the review by Mozilla and issue we overlooked. | ||
Line 367: | Line 691: | ||
|} | |} | ||
− | ====Remaining Test Reviews==== | + | ====Remaining Test Reviews [Deprecated]==== |
The following tests need investigation. Please add your name beside each test(s) you are assessing/reviewing/fixing/peer reviewing. | The following tests need investigation. Please add your name beside each test(s) you are assessing/reviewing/fixing/peer reviewing. | ||
Once the cause for rejection is clear, indicate it by placing an X in any of the columns and optionally filling out the comments column. Indicate the action taken to resolve the issue in the action taken column. Example actions include: | Once the cause for rejection is clear, indicate it by placing an X in any of the columns and optionally filling out the comments column. Indicate the action taken to resolve the issue in the action taken column. Example actions include: | ||
Line 389: | Line 713: | ||
| Action Taken | | Action Taken | ||
| Pull requested? | | Pull requested? | ||
+ | | bgcolor="red" style="text-align:center;" | Updated with Patchv3 | ||
| Comments | | Comments | ||
|- | |- | ||
− | | colspan= | + | | colspan=14 bgcolor="yellow" style="text-align:center;" | '''Ensure you have actually tested your test before sending a pull request''' |
|- | |- | ||
|- | |- | ||
Line 405: | Line 730: | ||
| | | | ||
|<s><b>IMPOSSIBLE IN MOCHITEST</b></s> | |<s><b>IMPOSSIBLE IN MOCHITEST</b></s> | ||
+ | | | ||
| | | | ||
|<s> | |<s> | ||
Line 425: | Line 751: | ||
|<s><b>FIXED, PEER REVIEWED, AND ADDED TO REPOSITORY</b></s> | |<s><b>FIXED, PEER REVIEWED, AND ADDED TO REPOSITORY</b></s> | ||
|X | |X | ||
+ | | | ||
| | | | ||
* <s>For Peer Reviewer: https://github.com/rhung/mozilla-central/blob/mouselock-tests/dom/tests/mochitest/mouselock/file_doubleLockCallBack.html</s> | * <s>For Peer Reviewer: https://github.com/rhung/mozilla-central/blob/mouselock-tests/dom/tests/mochitest/mouselock/file_doubleLockCallBack.html</s> | ||
|- | |- | ||
− | |110 | + | |<s>110</s> |
− | |test_exitMouselockOnLoseFocus.html | + | |<s>test_exitMouselockOnLoseFocus.html</s> |
− | |diogogmt | + | |<s>diogogmt</s> |
+ | | | ||
+ | |<s>X</s> | ||
+ | |<s>X</s> | ||
+ | |<s>X</s> | ||
| | | | ||
− | | | + | |<s>northWind</s> |
− | |X | + | |<s>X</s> |
− | | | + | |<s><b>FIXED, PEER REVIEWED, AND ADDED TO REPOSITORY</b></s> |
| | | | ||
− | |||
− | |||
− | |||
| | | | ||
| | | | ||
Line 445: | Line 773: | ||
|test_mousePos.html | |test_mousePos.html | ||
|diogogmt | |diogogmt | ||
+ | | | ||
| | | | ||
| | | | ||
Line 467: | Line 796: | ||
|- | |- | ||
− | |115 | + | |<s>115</s> |
− | |file_mouselocklost.html | + | |<s>file_mouselocklost.html</s> |
− | |diogogmt | + | |<s>diogogmt</s> |
| | | | ||
| | | | ||
Line 475: | Line 804: | ||
| | | | ||
| | | | ||
+ | |<s>rhung</s> | ||
| | | | ||
+ | |<s><b>PEER REVIEWED, AND ADDED TO REPOSITORY</b></s> | ||
| | | | ||
− | |||
| | | | ||
− | |Original file didn't need any changes. Passed all test criterias | + | |<s>Original file didn't need any changes. Passed all test criterias</s> |
− | + | <s>Awaiting peer review to cross it off the list</s> | |
− | Awaiting peer review to cross it off the list | ||
|- | |- | ||
− | |119 | + | |<s>119</s> |
− | |file_fullscreen.html | + | |<s>file_fullscreen.html</s> |
− | |northWind | + | |<s>northWind</s> |
− | |X | + | |<s>X</s> |
− | |X | + | |<s>X</s> |
− | |X | + | |<s>X</s> |
− | |X | + | |<s>X</s> |
+ | |<s>X</s> | ||
+ | |<s>rhung</s> | ||
+ | | | ||
+ | |<s><b>FIXED, PEER REVIEWED, AND ADDED TO REPOSITORY</b></s> | ||
|X | |X | ||
− | |||
| | | | ||
− | | | + | |<s>Lines</s> <s>49</s>, <s>66</s><s>, and 76 are too long. 76 in particular.</s> <s>Needs comment at the beginning to describe what the test does.</s> |
− | |||
− | |||
|- | |- | ||
− | |107 | + | |<s>107</s> |
− | | | + | |<s>file_screenXYclientXYConst.html</s> |
− | |diogogmt | + | |<s>diogogmt</s> |
− | |X | + | |<s>X</s> |
− | |X | + | |<s>X</s> |
| | | | ||
| | | | ||
| | | | ||
+ | |<s>anachid</s> | ||
| | | | ||
+ | |<s><b>FIXED, PEER REVIEWED, AND ADDED TO REPOSITORY</b></s> | ||
| | | | ||
− | |||
| | | | ||
− | |[https://github.com/diogogmt/mozilla-central/blob/mouselock-tests-constantXY/dom/tests/mochitest/mouselock/file_constantXY.html file_constantXY] | + | |<s>[https://github.com/diogogmt/mozilla-central/blob/mouselock-tests-constantXY/dom/tests/mochitest/mouselock/file_constantXY.html file_constantXY]</s> |
− | [https://github.com/diogogmt/mozilla-central/tree/mouselock-tests-constantXY branch] | + | <s>[https://github.com/diogogmt/mozilla-central/tree/mouselock-tests-constantXY branch]</s> |
+ | |||
+ | <s>* Anachid (formatting review on file commit version '''1f5fa3e0a5f7730f33eb5d3f7c51105fac4cb93d'''):</s> | ||
+ | <s>** Line 8 exceeds 80 character limit by 7 characters.</s> | ||
+ | <s>** Remove line 99 [ console.log("start"); ]</s> | ||
+ | <s>** Line 100 and 103 could be re-done to document.getElementById("div") instead of having a div variable, thereby removing variable space and risk of a bug</s> | ||
+ | <s>** Extra line breaks (should be removed) on line 74,77,78</s> | ||
|- | |- | ||
|108 | |108 | ||
|file_defaultUnlock.html | |file_defaultUnlock.html | ||
+ | |diogogmt | ||
| | | | ||
| | | | ||
Line 526: | Line 864: | ||
| | | | ||
|- | |- | ||
− | |103 | + | |<s>103</s> |
− | |file_DOMtree.html | + | |<s>file_DOMtree.html</s> |
− | | | + | |<s>diogogmt</s> |
− | |||
| | | | ||
| | | | ||
Line 535: | Line 872: | ||
| | | | ||
| | | | ||
+ | |<s>rhung</s> | ||
| | | | ||
+ | |<s><b>FIXED, PEER REVIEWED, AND ADDED TO REPOSITORY</b></s> | ||
| | | | ||
| | | | ||
Line 551: | Line 890: | ||
| | | | ||
|'''FIXING''' | |'''FIXING''' | ||
+ | | | ||
| | | | ||
| | | | ||
Line 565: | Line 905: | ||
| | | | ||
|'''PEER REVIEWED, SEE COMMENTS''' | |'''PEER REVIEWED, SEE COMMENTS''' | ||
+ | | | ||
| | | | ||
| | | | ||
Line 576: | Line 917: | ||
** 68: should add && document.mozFullScreenElement === outer to condition | ** 68: should add && document.mozFullScreenElement === outer to condition | ||
** 68: whitespace at end of line | ** 68: whitespace at end of line | ||
− | ** 78: looked at this briefly, are we sure that synthMs will cause the cursor to cross over the 'one' div? | + | ** <s>78: looked at this briefly, are we sure that synthMs will cause the cursor to cross over the 'one' div?</s> |
** 172: Col 45: "the" should be "a Left" | ** 172: Col 45: "the" should be "a Left" | ||
** 183-214: Why not testing for mousemove? | ** 183-214: Why not testing for mousemove? | ||
|- | |- | ||
|116 | |116 | ||
− | | | + | |file_movementXY.html |
− | | | + | |diogogmt |
− | |||
− | |||
| | | | ||
| | | | ||
Line 590: | Line 929: | ||
| | | | ||
| | | | ||
+ | | mjschranz | ||
| | | | ||
+ | |<b>REVIEWED, READY FOR PULL REQUEST</b> | ||
| | | | ||
| | | | ||
+ | |[https://github.com/diogogmt/mozilla-central/blob/mouselock-tests/dom/tests/mochitest/pointerlock/file_movementXY.html file_movementXY] | ||
+ | * mjschranz | ||
+ | ** As far as I can tell, there doesn't appear to be any styling errors. The tests run just fine and based on what I understand about it and what is listed in the comments the tests appear to function as intended. | ||
|- | |- | ||
− | |120 | + | |<s>120</s> |
− | |file_syntheticMouseEvent.html | + | |<s>file_syntheticMouseEvent.html</s> |
− | | | + | |<s>diogogmt</s> |
− | |||
| | | | ||
| | | | ||
Line 603: | Line 946: | ||
| | | | ||
| | | | ||
+ | |<s>rhung</s> | ||
| | | | ||
| | | | ||
| | | | ||
| | | | ||
+ | |<s>Merged with MouseEvents test</s> | ||
|- | |- | ||
|105 | |105 | ||
|file_targetOutOfFocus.html | |file_targetOutOfFocus.html | ||
+ | |diogogmt | ||
| | | | ||
| | | | ||
Line 615: | Line 961: | ||
| | | | ||
| | | | ||
+ | | mjschranz | ||
| | | | ||
+ | |<b>REVIEWED, SEE COMMENTS</b> | ||
| | | | ||
| | | | ||
− | | | + | |[https://github.com/diogogmt/mozilla-central/blob/mouselock-tests/dom/tests/mochitest/pointerlock/file_targetOutOfFocus.html file_targetOutOfFocus] |
− | + | * mjschranz | |
− | + | ** Line 41: the word focus is surrounded by single quotation marks, should be double ( "" ). | |
+ | ** No other styling issues seen. Tests run and appear to be functioning as per the specs. | ||
|- | |- | ||
|118 | |118 | ||
Line 627: | Line 976: | ||
| | | | ||
| | | | ||
+ | |X | ||
| | | | ||
− | | | + | |X |
| | | | ||
| | | | ||
Line 638: | Line 988: | ||
|121 | |121 | ||
|test_mouseLockable.html | |test_mouseLockable.html | ||
+ | (rename to file_mozPointerLock.html) | ||
| | | | ||
| | | | ||
− | | | + | |X |
− | | | + | |X |
− | | | + | |X |
+ | |X | ||
| | | | ||
| | | | ||
Line 658: | Line 1,010: | ||
|X | |X | ||
| | | | ||
+ | | mjschranz | ||
| | | | ||
+ | |<b>REVIEWED, SEE COMMENTS</b> | ||
| | | | ||
− | |||
| | | | ||
− | |[https://github.com/diogogmt/mozilla-central/blob/mouselock-tests | + | |[https://github.com/diogogmt/mozilla-central/blob/mouselock-tests/dom/tests/mochitest/pointerlock/file_nestedFullScreen.html file_nestedFullScreen] |
− | + | * mjschranz | |
+ | ** Line 48: isLocked method call is missing opening/closing brackets. The test still passes when FullScreenHarness is run however we obviously need to fix that and ensure it's still locked as intended. | ||
+ | ** No other styling issues seen. Fix previous error and double check all tests still pass when harness is run. | ||
|- | |- | ||
|<s>123</s> | |<s>123</s> | ||
Line 676: | Line 1,031: | ||
| | | | ||
|<s>'''FIXED, PEER REVIEWED, AND ADDED TO REPOSITORY'''</s> | |<s>'''FIXED, PEER REVIEWED, AND ADDED TO REPOSITORY'''</s> | ||
+ | | | ||
+ | | | ||
+ | | | ||
+ | |- | ||
+ | |<s>120</s> | ||
+ | |<s>file_differentDOM.html</s> | ||
+ | |<s>diogogmt</s> | ||
+ | | | ||
+ | | | ||
+ | | | ||
+ | | | ||
+ | | | ||
+ | |<s>rhung</s> | ||
+ | | | ||
+ | |<s>'''FIXED, PEER REVIEWED, AND ADDED TO REPOSITORY'''</s> | ||
+ | | | ||
| | | | ||
| | | |
Latest revision as of 00:09, 15 February 2012
Contents
Introduction
This is a working document for the implementation of the Mouse Lock API spec in Mozilla by students in David Humphrey's Mozilla Development class at Seneca College.
Please add, edit, correct, expand, etc. as necessary. This page should contain any links or other info we need.
Participants
While the project is primarily meant for students in DPS909/OSD600, feel free to join us if you want to work on things.
- David Humphrey (lead developer, professor, @humphd)
- Hasan Kamal-Al-Deen (tardy student, @NorthWind87)
- Matthew Schranz (Student, OSD600, @mjschranz)
- Yevgeniy Ivanchenko (Student, OSD600)
- Chris Gosselin (Student, OSD600)
- Anurag Bhatnagar (Student, DPS909, @anuragbh)
- Raymond Hung (Wannabe Developer, Student, @Raymond_Hung)
- Ausley Johnson(Student, OSD600)
- Jesse Silver (Student, OSD600)
- Ching Wei Tseng(Student, DPS909)
- Michelle Mendoza (Student, DPS909)
- Archana Sahota (Student, DPS909)
- Greg Krilov (Student, DPS909)
- Roman Hotin (Student, DPS909)
- Sergiu Ecob (Student, OSD600)
- Jordan Raffoul (Student)
- Hyungryul Chun (Student, DPS909)
- James Boelen (Masked Crusader, @jamesboelen)
- Jacky Siu (Student, OSD600)
- Abhishek Bhatnagar (Student, @abhishekToronto)
- Diogo Golovanevsky Monteiro (@diogogmt)
- Simon de Almeida(Student) (@simon661)
- Stanley Tsang (Student, DPS909)
- Denise Rigato (Student, DPS909)
- Qian (Ken) Xu (Student, DPS909)
- Moussa Tabcharani (Student, DPS909)
- Keyan Ren (Student, OSD600)
- Joseph Hughes (Student, OSD600)
Communication
Development work will be done using a combination of the following:
- In class discussions: 11:40-1:30 Tuesday and Thursday in TEL T2110
- IRC discussions: #seneca, #paladin
- Questions: ask and answer any questions here: Mouse Lock Implementation FAQ
- Blog Series: http://vocamus.net/dave/?cat=28
Tasks
Getting Started
- Clone our repo and build a debug version locally
- Get a https://bugzilla.mozilla.org account and CC yourself on the bug.
- Set a Watch on this page and the Q/A page so you know when things change.
- Break the spec down into an itemized list of things we need to do, tests we need to write, features we have to add, edge cases we have to worry about, demos we need to build, etc. Put the info into this page. We need to know everything we'll have to write and schedule when we'll do each bit.
- Blog about your work on this implementation
- Add questions/answers to Mouse Lock Implementation FAQ
High-Level Mouse Lock Implementation Tasks
Implementation
-
No mouse cursor is displayed when the mouse is locked- rhung -
MouseLockable DOM Implementation, navigator.pointer (Notes on MouseLock DOM Implementation Nov 13, 2011)- humph-
void lock (in Element target, optional in VoidCallback successCallback, optional in VoidCallback failureCallback);diogogmt, humph -
void unlock ();humph, diogogmt -
bool islocked ();- humph
-
-
Mouse Lock Platform ImplementationsJSilver999, humph-
Windows: SetCursorPos(x, y) -
OS X CGWarpMouseCursorPosition(CGPointMake(x, y)) -
Linux (GTK) gdk_display_warp_pointer (display, screen, x, y), add to http://mxr.mozilla.org/mozilla-central/source/widget/src/gtk2/nsWindow.cpp ??? - Mobile?
-
-
Investigate whether we can get movement information directly from OS</strke> - northWind -
mouselocklost event DOM Implementation- diogogmt -
Extend MouseEvent DOM implementation with movementX, movementY- humph, JSilver999 -
The browser must exit the mouse lock state if the user agent, window, or tab loses focus- diogogmt - <strike>Fixup NULLs being returned from lock() C++ implementation, should be NS* - Anachid
-
Mouse lock should only work when in Full Screen Mode- diogogmt, rhung -
Refactor nsIDOMNavigator changes for pointer attribute to be in separate interfacehumph -
Do we need to do conditional compilation for mouse lock?humph (not going to bother for now) -
When mouse lock is enabled clientX, clientY, screenX, and screenY must hold constant values as if the mouse did not move at all once mouse lock was entered.humph -
Freeze mouse pointer in centre of window when mouse lock is enabled (e.g., moving the mouse causes an event, but forces the mouse to go back to the original position).- JSilver999 -
"Events that require the concept of a mouse cursor must not be dispatched (for example: mouseover, mouseout)"- humph -
Figure out Mac Crash with Jesse's SynthesizeMouseMove changehumph -
When the locked element is removed from the DOM Tree, the mouse should be unlockeddiogogmt -
Save the screenX and screenY position before locking the mouse.- humph -
Reset the mouse position back to the original position when unlocking.- humph -
"When unlocked, the system cursor can exit and re-enter the user agent window. If it does so and the user agent was not the target of operating system mouse move events then the most recent mouse position will be unknown to the user agent and movementX/Y can not be computed and must be set to zero"diogogmt -
Trying to lock a locked element should fire the success callbackCloudScorpion -
Before locking the mouse check if the element is a DOM element and if it is in the DOM Treediogogmt -
Fix accurateness of mouse positioning on unlock() (should be the same point as when lock() was called). Currently works, but is offset. See nsEventStateManager::SetMouseLock.JSilver999 -
Restructure Lock method to do most of its operations in a separate thread.humph -
Fix license headers for new files to use proper MPL boilerplatehumph -
Do we need to add a user pref to enable/disable mouse lock? Nice to have, not blocking.northwind, mjschranz - "Once mouse lock is acquired, stop mouse events from being fired to other elements that are not locked (e.g., only fire to locked element)." Only the fullscreen element will get events. Need advice in review on how to do this properly.
Out of Scope Implementation
Because we are only implementing Mouse Lock for Fullscreen elements, some aspects of the spec can/must be put off until later. Other items below are simply not in scope for this first round of implementation.
- The ESC key should exit mouse lock. This will currently exit fullscreen, and therefore mouse lock - diogogmt
- "User agents may prompt for confirmation before locking, this preference may be saved as a content setting" How to deal with this? What UI do we use? See also, "Repeated escapes of mouse lock can signal user agent to not re-lock the mouse without more specific user intent gesture, e.g. similar to how Chrome suppresses repeated alert() calls"
- "The Mouse Lock API must exit the mouse lock state if the user agent, window, or tab loses focus"
- Clip the mouse so it doesn't leave the locked element with a mouse movement large enough to exceed its bounds. See:
- ClipCursor on Windows
- CGAssociateMouseAndMouseCursorPosition and CGGetLastMouseDelta on OS X
- XGrabPointer on Linux
Review Issues
-
Rewrite DOM node removal unlocking logic to use nsIMutatationObserverhumph, diogogmt -
Space, bracing, tabs, etc style nitshumph -
clientX, clientY, screenX, screenY values should follow spec as per scheib's commentdiogogmt -
Confirm |aEvent->lastRefPoint = nsIntPoint(bounds.width/2, bounds.height/2);| that / 2 is rightdiogogmt -
"nsCOMPtr<nsIContent> mMouseLockedElement; - Shouldn't this be a static member variable? Or how does the patch handle cases when the document which has iframe and mouse moves over those iframes." Probably wants a test for this case, too.diogogmt -
"nsDOMMouseLockable::ShouldLock...QI to nsINode and check IsInDoc()"diogogmt -
"nsRefPtr<nsMouseLockableRequest> request = new nsMouseLockableRequest(aSuccessCallback, aFailureCallback); -- You should store aTarget (QI'ed to nsINode or nsIContent or Element or nsIDOMEventTarget) so that when calling callback you push Cx to stack; nsCxPusher pusher: // defined in nsContentUtils.h NS_ENSURE_STATE(pusher.Push(target));"- humph -
"nsDOMMouseLockable looks like it should be cycle collectable"humph -
Break .pointer out of Navigator, and add nsIDOMMozNavigatorPointer (not sure if this name is best) as a new interface to Navigator, similar to nsIDOMMozNavigatorBattery.- humph, diogogmt -
Rename for moz* prefix. The API should be probably prefixed. So, navigator.mozPointer and all the interfaces should start with nsIDOMMoz. Similar to what webkit* is doing:- humph-
navigator.webkitPointer.unlock(); -
navigator.webkitPointer.lock(); -
navigator.webkitPointer.isLocked(); -
document.body.addEventListener("webkitpointerlocklost", ... -
mouseMoveEvent.webkitMovementX -
mouseMoveEvent.webkitMovementY
-
- Follow discussion unfolding here for other changes: http://lists.w3.org/Archives/Public/public-webapps/2011OctDec/1558.html
- Finish tests, updated to changes above.
-
Update patch to trunk- humph - Figure out why DOMMouseScroll events aren't being retargeted to the locked element
- Look into roc's suggestion from bug 722449 comment 8, namely, using getBoundingClientRect vs. GetPrimaryFrame()
- Maybe use nsStubMutationObserver instead of nsIMutationObserver?
- Extensions to the Document Interface. Add pointerLockElement attribute and exitPointerLock method - diogogmt
- Update Element interface to have requestPointerLock - diogogmt
-
Dispatch pointerlockchange or pointerlockerror events instead of firing a callback when pointer gets locked- diogogmt
- Update mochitests to use new pointerlock API
- Add expect number of tests to mochitests
- Update patch to trunk
Tests
The central repo with all the tests are located here. For more information on how to make mochitests and how to send the pull request to the appropriate person, check out the Mochitest FAQ List tests we need below:
Id | Description | Test Author(s) | Test File |
1 | hchun | file_limitlessScroll.html | |
2 | hchun | file_limitlessScroll.html | |
3 | "no mouse cursor is displayed" -- mouse cursor should be hidden while locked | dvillase | |
4 | abhatnagar | test_navigatorPointer.html | |
5 | abhatnagar | test_mouseLockableHasRequiredMethods.html | |
6 | JSilver999 | file_fullscreen.html | |
7 | JSilver999 | file_fullscreen.html | |
8 | JSilver999 | file_fullscreen.html | |
9 | "Mouse lock must succeed only if the window is in focus" | johnno | |
10 | "Mouse lock must succeed only if...the user-agent is the active application of the operating system" | johnno | |
11 | johnno | file_targetOutOfFocus.html | |
12 | Anachid | file_DOMtree.html | |
13 | Anachid | file_DOMtree.html | |
14 | jboelen | test_doubleLockCallBack.html | |
15 | jboelen | DEFUNCT | |
16 | CloudScorpion | file_defaultUnlock.html | |
17 | rhung | file_MouseEvents.html | |
18 | rhung | file_MouseEvents.html | |
19 | "Movement and button presses of the mouse must not cause the window to lose focus" - | ||
20 | Tentacle | file_syntheticMouseEvent.html | |
21 | abhatnagar1 | No test written for this case specifically but file_fullscreen.html covers it. | |
22 | "[Upon unlock() t]he system mouse cursor must be displayed again and positioned at the same location that it was when mouse lock was entered (the same location that is reported in screenX/Y when the mouse is locked)" | dvillase | |
23 | stsang | file_mouselocklost.html | |
24 | KeyR, JSilver999 | file_movement.html | |
25 | KeyR, JSilver999 | file_movement.html | |
26 | KeyR, JSilver999 | file_movement.html | |
27 | moussa1 | test_mousePos.html | |
28 | jbraffoul | file_constantxy.html | |
29 | drigato | test_exitMouselockOnLoseFocus.html | |
30 | Test to make sure that mouse lock only occurs when an element is in full screen mode (not F11 or done via the menus). This includes:
|
jsiu3 | |
31 | stsang | file_mouselocklost.html | |
32 | northWind | file_unlockOnUnload.html |
Reviewing Tests
There are a series of common mistakes in the tests that need to get fixed. Here are some of them:
- Need to clean-up tabs vs. spaces and indentation issues (2-spaces per tab) in many test files. Use https://github.com/einars/js-beautify. This is a test that is formatted correctly, in terms of indentation and spaces vs. tabs.
- No line of code should be 80 characters or longer--break them so they are under 80
- No Windows end-of-lines, use Unix end-of-lines
- No whitespace at the end of a line
- Run your test code through http://www.jshint.com/. Note, it will complain about unknown globals like SimpleTest. Make sure the rest of the JavaScript is good.
- Remove unnecessary comments. Only things that explain the test.
- If you need constants, use const instead of var
- Don't use variables if they aren't needed. For example, don't introduce a variable to store a value, only to pass it into ok() or is(). Just test the expression in ok() or is().
- Make sure your JavaScript follows proper naming: goodVariableName, bad_variable_name, badvariablename;
- Tests should follow the template as closely as makes sense: http://pastebin.com/vxmsepVh
- Tests should include a simple comment block describing what is being tested
- Only use
SimpleTest.waitForFocus()
if you really need it. - Be consistent with "..." vs. '...' for strings. Pick one and use it throughout the file.
- Remove
console.log()
- Prefer
document.body
todocument.getElementsByTagName('body')[0]
- Braces on the same line:
if (...) {\n
and} else {\n
- An error message for is() or ok() of "Error message" is not acceptable. Make sure you understand the failure.
- If you need to actually lock the mouse, you'll need
SpecialPowers.setBoolPref("full-screen-api.allow-trusted-requests-only", false);
to allow non-user initiated fullscreen mode (i.e., normally it requires a user to click a button or trigger some other event). - If you're testing that variable foo or expression fooFunction() are true, use ok(), not is():
ok(foo, "Error message if not true.");
orok(fooFunction(), "Error message if function returns false");
. Don't dois(foo, true, "Error message if false.");
- Make sure code that relies on things happening in asynchronous code gets called in a callback or event handler, not on the next line. For example, if you call navigator.pointer.lock(), you can't check navigator.pointer.islocked() on the next line, you need to use the successCallback/errorCallback. Same for focus or blur calls and events. Actions that trigger something happening in the future often need a callback or event handler.
- Prefer <code>i++; to
i+=1;
- Many tests require the harness and focus, ensure that those tests are correctly written ie:
- For Harness: See #Using the Test Harness
- For Focus: See #Waiting For Focus
One or more tests have to deal with switching the lock between multiple elements, and the spec is changing on that front.
New Test Reviews [Updated 1/19/2012]
This is a new list of test. The following tests will need to confirm to the new prefixes in accordance to Patchv3 of Bug 633602. Please add your name beside each test(s) you are assessing/reviewing/fixing/peer reviewing. Once the cause for rejection is clear, indicate it by placing an X in any of the columns and optionally filling out the comments column. Indicate the action taken to resolve the issue in the action taken column. Example actions include:
- DELETED
- FIXED
Other actions are also acceptable where appropriate. Once fixed, please pull request to rhung's mouselock-tests branch and indicate that you've done so in the pull requested column. Ensure that you add [POST-SUBMIT] to the name of your pull request.
Num | Name | Primary Reviewer | Timing issues? | Needs Harness? | Needs focus? | Formatting issues? | Other? (Indicate in comments) | Peer Reviewers | Peer reviewed? | Action Taken | Pull requested? | Updated with Patchv3 | Comments |
Ensure you have actually tested your test before sending a pull request | |||||||||||||
1 | test_FullScreenHarness.html | Anachid | X (no action) | X (no action) | X (no action) | X (no action) | - | FIXED | X | ||||
2 | file_DOMtree.html | Anachid | X (fixed) | X (fixed) | X (fixed) | X (no action) | - | FIXED | X | ||||
3 | file_nestedFullScreen.html | Anachid | X (fixed) | X (fixed) | X (fixed) | X (no action) | - | FIXED | X | ||||
4 | file_differentDOM.html | Anachid | X (no action) | X (no action) | X (no action) | X (no action) | - | FIXED | X | ||||
5 | file_fullscreen.html | Anachid | X | X | X | X | - | FIXED | X | Test needs to completely re-done. Cases like
| |||
6 | file_doubleLockCallBack.html | Anachid | X | X | X | X | - | FIXED | X | ||||
7 | file_unlockOnUnload.html | Anachid | X | X | X | X | - | FIXED | X | ||||
8 | file_exitMouseLockOnLoseFocus.html | Anachid | X | X | X | X | - | FIXED | X | ||||
9 | test_mousePos.html | rhung | Doesn't seem like it can be done via mochitest | REMOVED | As mentioned in the previous comments, synthesizeMouse cannot be moved beyond the browser. It might need to be a litmus test. | ||||||||
10 | file_mouselocklost.html | Anachid | X | X | X | X | - | FIXED | X | Removed some test that were repeating. Otherwise, it works | |||
11 | file_movementXY.html | rhung | X | X | X | X | - | Fixing | X | ||||
12 | file_targetOutOfFocus.html | rhung | X | X | X | X (fixed) | - | FIXED | X | Minor changes, mostly styling. | |||
13 | test_mouseLockable.html | rhung | X | X | X | X | - | FIXED & RENAMED | X | Updated to test_MozPointerLock.html | |||
14 | file_MouseEvents.html | rhung | X | X | X | X | Leak issue might be platform specific. | FIXED? | X | Works without chance of hanging or random success and failures. | |||
15 | file_limitlessScroll.html | rhung | MERGED WITH file_movementXY | ||||||||||
16 | file_defaultUnlock.html | rhung | FIXED | X | |||||||||
17 | file_userPref.html | rhung | FIXED | X | |||||||||
18 | file_constantXY.html | rhung | FIXING | X | Needs proper error messages, perhaps formatting issues. Test runs fine though. |
New Tests (not sure if it will work on mochitest, and the test will possibly fail as dave is working on this patch)
- When there is a parent element with multiple child element, have a test to check that all mouse events fired inside the child element are suppressed and is captured by the parent element. (this may also include the scenario where there is an iframe in the parent element)
- A Test to check that repeated move of the mouse to a certain direction that will exceed the screen size does not break the mouselock (this may conflict with file_movementXY.html and we may need to add more test cases)
Changes To Be Applied To Submitted Tests [Deprecated]
The following table is used to maintain a small note on the task for each test. It will include note from the review by Mozilla and issue we overlooked.
Num | Test Name | Reviewer | Notes | ||||
Please add your name to the section you want to review and fix | |||||||
Fill out the Notes section for any issues that are outstanding | |||||||
1 | test_DOMtree.html |
| |||||
2 | test_MouseEvents.html |
| |||||
3 |
Remaining Test Reviews [Deprecated]
The following tests need investigation. Please add your name beside each test(s) you are assessing/reviewing/fixing/peer reviewing. Once the cause for rejection is clear, indicate it by placing an X in any of the columns and optionally filling out the comments column. Indicate the action taken to resolve the issue in the action taken column. Example actions include:
- DELETED
- FIXED
Other actions are also acceptable where appropriate. Once fixed, please pull request to rhung's mouselock-tests branch and indicate that you've done so in the pull requested column. Ensure that you add [POST-SUBMIT] to the name of your pull request.
Num | Name | Primary Reviewer | Timing issues? | Needs Harness? | Needs focus? | Formatting issues? | Other? (Indicate in comments) | Peer Reviewers | Peer reviewed? | Action Taken | Pull requested? | Updated with Patchv3 | Comments |
Ensure you have actually tested your test before sending a pull request | |||||||||||||
| |||||||||||||
X | X | X | X | X | |||||||||
114 | test_mousePos.html | diogogmt | A summary of the test:
The problem is that synthesizeMouse doesn't move the mouse outside the boundaries of the specified element. If the X/Y coords passed to synthesizeMouse are bigger than the X/Y of the element receiving the event, it won't move the cursor. I'm logging all the screen/client/movementXY values, and if the coords passed to synthesizeMouse are bigger than the boundaries of the element receiving the mousemove, the test will break, since the mouse is never moved.
code
| ||||||||||
| |||||||||||||
X | |||||||||||||
| |||||||||||||
108 | file_defaultUnlock.html | diogogmt | |||||||||||
112 | file_limitlessScroll.html | rhung | X | rhung: Needs to be redone to take into account where the mouse currently is rather than sending it a totally new point. | FIXING | ||||||||
104 | file_MouseEvents.html | rhung | X | X | northWind | PEER REVIEWED, SEE COMMENTS |
| ||||||
116 | file_movementXY.html | diogogmt | mjschranz | REVIEWED, READY FOR PULL REQUEST | file_movementXY
| ||||||||
105 | file_targetOutOfFocus.html | diogogmt | mjschranz | REVIEWED, SEE COMMENTS | file_targetOutOfFocus
| ||||||||
118 | file_userPref.html | X | X | ||||||||||
121 | test_mouseLockable.html
(rename to file_mozPointerLock.html) |
X | X | X | X | ||||||||
122 | file_nestedFullScreen.html | diogogmt | X | mjschranz | REVIEWED, SEE COMMENTS | file_nestedFullScreen
| |||||||
X | |||||||||||||
Waiting For Focus
Many tests will intermittently hang if they don't wait for focus. To have a test wait for focus, ensure that the test incorporates the following template:
<!-- snip --> <body onload="runTests();"> <!-- other tags that you may need go here --> <script> function runTests() { SimpleTest.waitForExplicitFinish(); SimpleTest.waitForFocus(function() { // Test code goes here }); } </script> </body> <!-- /snip -->
Using the Test Harness
Note: This harness assumes that your test runs properly as a stand-alone application and is not being used as one of the test amongst others.
The problem with the test timing out is due to the fact that the Mochitest iframe does not allow fullscreen from the html code inside the frame. To solve this problem, it needs a small work around like using the harness.
To make use of the harness, you will need to add the mouselock_util.js file to your test html file. I would recommend renaming your file to file_something.html (vs. test_something.html), as this denotes that this not a direct test under the mochitest.
The same applies to the Makefile.in. Change the filename to their appropriate name in the Makefile.in
Thus, add the following line, AFTER the EventUtil.js and SimpleTest.js script tag
<script type="application/javascript" src="mouselock_util.js"></script>
Another, thing that needs to be done is to add your file to the gTestFiles array inside the test_FullScreenHarness.html file
After that, do a run of the whole mochitest directory on your machine before doing a pull request. (use this):
TEST_PATH=dom/tests/mochitest/mouselock/ make -C $(replace with OBJDIR) mochitest-plain
Finally, since the fullscreen needs the harness, the harness will provide the special powers. So please do NOT add any special powers in your child files.
Demos, Docs, Other
- Proper IDL documentation for navigator.pointer (see example in https://mxr.mozilla.org/mozilla-central/source/dom/interfaces/html/nsIDOMHTMLElement.idl#103), MouseLockable and its methods, MouseLockLost event, etc.
- Write a JavaScript library to somehow combine element.mozRequestFullScreen() and navigator.pointer.lock(). It would be good to hide the complexities of doing fullscreen then locking in a single API call.
var canvas = document.getElementById('canvas'); // Will put element into fullscreen then try to do mouse lock var lock = acquireMouseLock({ element: canvas, onLock: function() { // OPTIONAL // fullscreen + mouse lock are now setup for element }, onUnlock: function() { // OPTIONAL // the user or browser took us out of fullscreen/mouse lock }, onError: function() { // OPTIONAL // there was an error getting fullscreen/mouse lock }, onMovemennt: function(e) { // OPTIONAL // movementX and movementY for mousemove are passed in var deltaX = e.movementX; var deltaY = e.movementY; } }); ... lock.islocked; // true or false lock.unlock();
- Mouse lock specification fix requests
- Convert Rescue Fox to use Mouse Lock, see https://github.com/mozilla/rescuefox
-
Convert http://cjcliffe.github.com/CubicVR.js/cubicvr/samples/fps_demo/level1.html to use Mouse Lock- JSilver999 - Create a tutorial on how to use Mouse Lock, with code examples
- Add demo pages to gh-pages branch
- Review https://developer.mozilla.org/en/API/Mouse_Lock_API for correctness with spec + our implementation
Resources
- Spec Document: http://dvcs.w3.org/hg/webevents/raw-file/default/mouse-lock.html
- Implementation Repo: https://github.com/humphd/mozilla-central on the mouselock branch.
- Mozilla Bug: https://bugzilla.mozilla.org/show_bug.cgi?id=633602
- WebKit Bug: https://bugs.webkit.org/show_bug.cgi?id=68468
- Chromium Bug: http://code.google.com/p/chromium/issues/detail?id=72754
- Points of Interest: http://zenit.senecac.on.ca/wiki/index.php/Mouse_Lock_API_Points_of_Interest
- Mochitest: https://developer.mozilla.org/en/Mochitest
- Mochitest FAQ: http://zenit.senecac.on.ca/wiki/index.php/Mochitest_FAQ
- Mozilla Cross Reference: http://mxr.mozilla.org/mozilla-central/
- MouseLock Demos Link: http://humphd.github.com/mozilla-central/mouselock/index.html
- MouseLock Stage Build download: http://stage.mozilla.org/pub/mozilla.org/firefox/try-builds/david.humphrey@senecac.on.ca-f4e5849f1ae7/