Difference between revisions of "Mouse Lock API Test Tracker"
(→Review Issues [This list consists of older issues that were resolved]) |
|||
(15 intermediate revisions by the same user not shown) | |||
Line 27: | Line 27: | ||
* Finish tests, updated to changes above. | * Finish tests, updated to changes above. | ||
* <s>Update patch to trunk</s> - humph | * <s>Update patch to trunk</s> - humph | ||
− | * Figure out why DOMMouseScroll events aren't being retargeted to the locked element | + | * <s>Figure out why DOMMouseScroll events aren't being retargeted to the locked element</s> - humph, diogogmt |
− | * Look into roc's suggestion from bug 722449 comment 8, namely, using getBoundingClientRect vs. GetPrimaryFrame() | + | * <s>Look into roc's suggestion from bug 722449 comment 8, namely, using getBoundingClientRect vs. GetPrimaryFrame()</s> - diogogmt |
− | * Maybe use nsStubMutationObserver instead of nsIMutationObserver? | + | * <s>Maybe use nsStubMutationObserver instead of nsIMutationObserver?</s> - humph |
− | * Extensions to the Document Interface. Add pointerLockElement attribute and exitPointerLock method - diogogmt | + | * <s>Extensions to the Document Interface. Add pointerLockElement attribute and exitPointerLock method</s> - diogogmt |
− | * Update Element interface to have requestPointerLock - diogogmt | + | * <s>Update Element interface to have requestPointerLock</s> - diogogmt |
* <s>Dispatch pointerlockchange or pointerlockerror events instead of firing a callback when pointer gets locked</s> - 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 | + | * <s>Update mochitests to use new pointerlock API</s> - diogogmt |
* Add expect number of tests to mochitests | * Add expect number of tests to mochitests | ||
− | * Update patch to trunk | + | * Update patch to trunk - diogogmt |
+ | ==Mochitests for new PointerLock API== | ||
+ | |||
+ | To have consistency between all the tests they were updated to the following: | ||
+ | * Through out the test flags are set to represent success or failure | ||
+ | * runTests function will run the assertions using the flags | ||
+ | * All assertions are located in runTests() | ||
+ | * body onload calls start() | ||
+ | * start() waits for focus then start running the test | ||
+ | * If test uses fullscreen API, then runTests() and SimpleTest.finish() are called when fullscreen mode is cancelled | ||
+ | |||
+ | Structure of the tests(Top-Bottom): | ||
+ | # Test description | ||
+ | # SpecialPowers(if nedded) | ||
+ | # SimpleTest.waitForExplicitFinish() | ||
+ | # var declarations | ||
+ | # function runTests() | ||
+ | # Some code specific to the test | ||
+ | # pointerlockchange event listener | ||
+ | # pointerlockerror event listener | ||
+ | # mozfullscreenchange event listener | ||
+ | ## runTests() | ||
+ | ## SimpleTest.finish() | ||
+ | # function start() | ||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
{| border=1 style="border: 1px solid darkgray;" | {| border=1 style="border: 1px solid darkgray;" | ||
| Num | | Num | ||
| Name | | Name | ||
− | | | + | | Reviewers |
− | | | + | | Updated? |
− | + | | Reviewed? | |
− | |||
− | | | ||
− | |||
− | |||
− | |||
− | |||
| Comments | | Comments | ||
|- | |- | ||
− | | | + | | 1 |
+ | | <s>test_FullScreenHarness.html</s> | ||
+ | | diogogmt | ||
+ | | Removed | ||
+ | | | ||
+ | | Not needed anymore since now it's possible to request fullscreen on the mochitest iframe | ||
+ | |- | ||
+ | |||
+ | | 2 | ||
+ | | <s>file_DOMtree.html</s> | ||
+ | | diogogmt | ||
+ | | Removed | ||
+ | | | ||
+ | |Split between test_withoutDOM and test_removedFromDOM | ||
|- | |- | ||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | | | + | | 2 |
− | + | | <b>test_withoutDOM.html</b> | |
− | + | | diogogmt | |
− | | | + | | New |
− | | | + | | |
− | | | + | | Checks if element is attached to the DOM Tree before locking the pointer |
|- | |- | ||
+ | |||
| 2 | | 2 | ||
− | | | + | | test_removedFromDOM.html</b> |
− | | | + | | diogogmt |
− | | | + | | New |
− | |||
− | |||
− | |||
− | |||
| | | | ||
− | + | | Checks if pointer is unlocked when element is removed from DOM Tree | |
− | |||
− | | | ||
− | |||
− | |||
|- | |- | ||
+ | |||
| 3 | | 3 | ||
− | | file_nestedFullScreen.html | + | | <s>file_nestedFullScreen.html</s><br /><b>test_nestedFullScreen.html</b> |
− | | | + | | diogogmt |
− | + | | Upated | |
− | |||
− | |||
− | |||
− | | | ||
| | | | ||
+ | | Requesting fullscreen on a child element of the element with the pointer locked should unlock the pointer | ||
+ | |- | ||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
| 4 | | 4 | ||
− | | file_differentDOM.html | + | | <s>file_differentDOM.html</s> |
− | | | + | | diogogmt |
− | | | + | | Removed |
− | |||
− | |||
− | |||
− | |||
| | | | ||
+ | | On the old spec the pointer object was attached to the navigator, so there was a possibility that an element belonging to a different DOM could request pointerlock. However, in the new API the element requests pointerlock on it self making this test redundant. | ||
+ | |- | ||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
| 5 | | 5 | ||
− | | file_fullscreen.html | + | | <s>file_fullscreen.html</s> |
− | | | + | | diogogmt |
− | | | + | | Removed |
− | |||
− | |||
− | |||
− | |||
| | | | ||
+ | | Tested components of the old spec and had some redundant tests | ||
+ | |- | ||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
| 6 | | 6 | ||
− | | file_doubleLockCallBack.html | + | | <s>file_doubleLockCallBack.html</s><br /><b>test_doubleLock.html</b> |
− | | | + | | diogogmt |
− | + | | Renamed<br/>Updated | |
− | + | | | |
− | + | | If element requests pointerlock on itself while in pointerlock state mozpointerlockchange event should be dispatched | |
− | |||
− | |||
− | |||
− | |||
− | |||
− | | | ||
− | | | ||
− | | | ||
|- | |- | ||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
| 8 | | 8 | ||
− | | file_exitMouseLockOnLoseFocus.html | + | | <s>file_exitMouseLockOnLoseFocus.htm</s><br /><b>test_looseFocusWindow.html</b> |
− | | | + | | diogogmt |
− | + | | Renamed<br />Updated | |
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | | | ||
| | | | ||
− | | | + | | If element has the pointer locked and window loses focus pointer should be unlocked |
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
|- | |- | ||
+ | |||
| 10 | | 10 | ||
− | | | + | | <s>file_pointerlocklost.html</s> |
− | | | + | | diogogmt |
− | | | + | | Removed |
− | |||
− | |||
− | |||
− | |||
| | | | ||
+ | | On the new updated spec pointerlocklost event doesn't exist anymore | ||
+ | |- | ||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
| 11 | | 11 | ||
− | | file_movementXY.html | + | | <s>file_movementXY.html</s><br /><b>test_movementXY.html</b> |
− | | | + | | diogogmt |
− | | | + | | Renamed<br />Updated |
− | + | | | |
− | + | | Checks if mozMovementX and mozMovementY are present in the mouse event object. It also checks the values for mozMovementXY. They should be equal to the current screenXY minus the last screenXY | |
− | |||
− | |||
− | |||
− | |||
− | | | ||
− | | | ||
− | |||
− | |||
|- | |- | ||
+ | |||
| 12 | | 12 | ||
− | | file_targetOutOfFocus.html | + | | <s>file_targetOutOfFocus.html</s><br /><b>test_targetOutOfFocus</b> |
− | + | | diogogmt | |
− | | | + | | Renamed<br />Updated |
− | | | + | | |
− | + | | Element doesn't need to have focus to request pointer lock | |
− | | | ||
− | | | ||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
|- | |- | ||
+ | |||
| 13 | | 13 | ||
− | | | + | | <s>test_MozPointerLock.html</s> |
− | | | + | | diogogmt |
− | + | | Removed | |
− | + | | | |
− | + | | On the new updated spec we don't have a MozPointerLock object anymore | |
− | | | ||
− | | | ||
− | | | ||
− | |||
− | |||
− | |||
− | |||
− | |||
|- | |- | ||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
| 15 | | 15 | ||
− | | file_cursorPosEvents.html | + | | <s>file_cursorPosEvents.html</s><br /><b>test_suppressSomeMouseEvents</b> |
− | | | + | | diogogmt |
− | | | + | | Renamed<br />Updated |
− | |||
− | |||
− | |||
− | |||
| | | | ||
− | + | | Test will check to make sure that the following mouse events are no longer executed in pointer lock. - mouseover, mouseout, mouseenter, mouseleave | |
− | |||
− | | | ||
− | |||
− | |||
|- | |- | ||
− | | | + | |
− | | | + | | 17 |
− | | | + | | <s>file_defaultUnlock.html</s><br /><b>test_escapeKey</b> |
− | + | | diogogmt | |
− | + | | Renamed<br />Updated | |
− | |||
− | |||
− | |||
− | |||
− | | | ||
| | | | ||
− | | | + | | Escape key should unlock the pointer |
|- | |- | ||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
| 18 | | 18 | ||
− | | file_userPref.html | + | | <s>file_userPref.html</s><br /><b>test_pointerLockPref.html</b> |
− | | | + | | diogogmt |
− | + | | Renamed<br />Updated | |
− | |||
− | |||
− | |||
− | | | ||
| | | | ||
− | + | | Tests full-screen-api.pointer-lock pref | |
− | |||
− | | | ||
− | |||
− | |||
− | |||
− | |||
|- | |- | ||
+ | |||
| 19 | | 19 | ||
− | | file_constantXY.html | + | | <s>file_constantXY.html</s><br /><b>test_screenClientXYConst.html</b> |
− | | | + | | diogogmt |
− | | | + | | Renamed<br />Updated |
− | | | + | | |
− | | | + | | Confirm that screenX/Y and clientX/Y are constants when the pointer is locked. |
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
|- | |- | ||
+ | |||
| 20 | | 20 | ||
− | | file_pointerLockCSSDisplay.html | + | | <s>file_pointerLockCSSDisplay.html</s><br /><b>test_cssDisplayProperty.html</b> |
− | | | + | | diogogmt |
− | | | + | | Renamed<br />Updated |
− | + | | | |
− | | | + | | Pointer should not be locked to an element with css property display is set to none |
− | | | ||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
|- | |- | ||
+ | |||
| 21 | | 21 | ||
− | | | + | | <s>file_retargetMouseEvents.html</s><br /><b>test_retargetMouseEvents</b> |
− | | | + | | diogogmt |
− | | | + | | Renamed<br />Updated |
− | | | + | | |
− | | | + | | Retarget mouse events to the locked element |
− | |||
− | |||
− | |||
− | |||
− | |||
− | |||
|- | |- | ||
|} | |} | ||
− | |||
− | |||
==Test [as of 31st January, 2012]== | ==Test [as of 31st January, 2012]== |
Latest revision as of 14:45, 29 February 2012
Contents
Mouse Lock API Test
The Mouse Lock Test had changed considerably since the start of the MouseLock Implementation from the Fall 2011 Semester. As such, a new page is required to track the test properly. A link to the old test tracker is available here. The tests are coded by three person:
Review Issues [This list consists of older issues that were resolved]
-
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- humph, diogogmt
-
Look into roc's suggestion from bug 722449 comment 8, namely, using getBoundingClientRect vs. GetPrimaryFrame()- diogogmt
-
Maybe use nsStubMutationObserver instead of nsIMutationObserver?- humph
-
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- diogogmt
- Add expect number of tests to mochitests
- Update patch to trunk - diogogmt
Mochitests for new PointerLock API
To have consistency between all the tests they were updated to the following:
- Through out the test flags are set to represent success or failure
- runTests function will run the assertions using the flags
- All assertions are located in runTests()
- body onload calls start()
- start() waits for focus then start running the test
- If test uses fullscreen API, then runTests() and SimpleTest.finish() are called when fullscreen mode is cancelled
Structure of the tests(Top-Bottom):
- Test description
- SpecialPowers(if nedded)
- SimpleTest.waitForExplicitFinish()
- var declarations
- function runTests()
- Some code specific to the test
- pointerlockchange event listener
- pointerlockerror event listener
- mozfullscreenchange event listener
- runTests()
- SimpleTest.finish()
- function start()
Num | Name | Reviewers | Updated? | Reviewed? | Comments |
1 | |
diogogmt | Removed | Not needed anymore since now it's possible to request fullscreen on the mochitest iframe | |
2 | |
diogogmt | Removed | Split between test_withoutDOM and test_removedFromDOM | |
2 | test_withoutDOM.html | diogogmt | New | Checks if element is attached to the DOM Tree before locking the pointer | |
2 | test_removedFromDOM.html</b> | diogogmt | New | Checks if pointer is unlocked when element is removed from DOM Tree | |
3 | test_nestedFullScreen.html |
diogogmt | Upated | Requesting fullscreen on a child element of the element with the pointer locked should unlock the pointer | |
4 | |
diogogmt | Removed | On the old spec the pointer object was attached to the navigator, so there was a possibility that an element belonging to a different DOM could request pointerlock. However, in the new API the element requests pointerlock on it self making this test redundant. | |
5 | |
diogogmt | Removed | Tested components of the old spec and had some redundant tests | |
6 | test_doubleLock.html |
diogogmt | Renamed Updated |
If element requests pointerlock on itself while in pointerlock state mozpointerlockchange event should be dispatched | |
8 | test_looseFocusWindow.html |
diogogmt | Renamed Updated |
If element has the pointer locked and window loses focus pointer should be unlocked | |
10 | |
diogogmt | Removed | On the new updated spec pointerlocklost event doesn't exist anymore | |
11 | test_movementXY.html |
diogogmt | Renamed Updated |
Checks if mozMovementX and mozMovementY are present in the mouse event object. It also checks the values for mozMovementXY. They should be equal to the current screenXY minus the last screenXY | |
12 | test_targetOutOfFocus |
diogogmt | Renamed Updated |
Element doesn't need to have focus to request pointer lock | |
13 | |
diogogmt | Removed | On the new updated spec we don't have a MozPointerLock object anymore | |
15 | test_suppressSomeMouseEvents |
diogogmt | Renamed Updated |
Test will check to make sure that the following mouse events are no longer executed in pointer lock. - mouseover, mouseout, mouseenter, mouseleave | |
17 | test_escapeKey |
diogogmt | Renamed Updated |
Escape key should unlock the pointer | |
18 | test_pointerLockPref.html |
diogogmt | Renamed Updated |
Tests full-screen-api.pointer-lock pref | |
19 | test_screenClientXYConst.html |
diogogmt | Renamed Updated |
Confirm that screenX/Y and clientX/Y are constants when the pointer is locked. | |
20 | test_cssDisplayProperty.html |
diogogmt | Renamed Updated |
Pointer should not be locked to an element with css property display is set to none | |
21 | test_retargetMouseEvents |
diogogmt | Renamed Updated |
Retarget mouse events to the locked element |
Test [as of 31st January, 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. 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 Steven's (Anachid's mouselock) 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) | Action Taken | Pull requested? | Updated with Patchv3 | Comments | ||
Add my repo as a "remote" and 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) | - |
|
Merged | X | No issues | ||
2 | file_DOMtree.html | Anachid | X (fixed) | X (fixed) | X (fixed) | X (no action) | - |
|
Merged | X | No issues | ||
3 | file_nestedFullScreen.html | Anachid | X (fixed) | X (fixed) | X (fixed) | X (no action) | - |
|
Merged | X | Had timing issues | ||
4 | file_differentDOM.html | Anachid | X (no action) | X (no action) | X (no action) | X (no action) | - |
|
Merged | X | Removed style "display: none" as this causes a crash | ||
5 | file_fullscreen.html | Anachid | X | X | X | X | - |
|
Merged | X | Test needs to completely re-done. Cases like
| ||
6 | file_doubleLockCallBack.html | Anachid | X | X | X | X | - |
|
Merged | X | Re-formatted for consistencies | ||
7 | file_unlockOnUnload.html | Anachid | X | X | X | X | - |
|
Merged | X | Fixed formatting issues | ||
8 | file_exitMouseLockOnLoseFocus.html | Anachid | X | X | X | X | - |
|
Merged | X | Re-formatted for consistencies | ||
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 | - |
|
Merged | X |
| ||
11 | file_movementXY.html | rhung | X | X | X | X | - | Tentative Fix | Merged | X | This test have multiple parts
| ||
12 | file_targetOutOfFocus.html | Anachid |
X | X | X | X (fixed) | - |
|
Merged | X |
| ||
13 | test_mouseLockable.html | rhung | X | X | X | X | - |
|
Merged | X | Updated to test_MozPointerLock.html | ||
14 | file_MouseEvents.html | rhung | X | X | X | X | no longer a platform issue as it happens on linux as well - Anachid |
FIXING | X |
| |||
15 | file_cursorPosEvents.html | rhung | X | X | X | X | - |
|
Merged | X | stripped out from file_MouseEvents.html | ||
16 | file_limitlessScroll.html | rhung | MERGED WITH file_movementXY | See file_movementXY.html for comment | |||||||||
17 | file_defaultUnlock.html | Anachid |
X | X | X | X | read comments |
|
Merged | X | Changed ownership to Anachid -
| ||
18 | file_userPref.html | rhung |
|
Merged | X | Anachid -
| |||||||
19 | file_constantXY.html | rhung | X | X | X | X | - |
|
Merged | X | Needs proper error messages, perhaps formatting issues. Test runs fine though. | ||
20 | file_pointerLockCSSDisplay.html | Anachid | X | X | X | X | - |
|
Merged | X | New test after crash reported here
| ||
21 | [unknown filename] | Diogogmt | [Unknown] | Test on checking that events inside a locked element are re-targeted to the element in lock. |
Possible Test Cases
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) - Being worked on by Diogogmt
- 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) - Should be in file_movementXY.html