| Summary: | REGRESSION: pageX and pageY are both 0 for events created with initMouseEvent | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Darin Adler <darin> | ||||||||||
| Component: | DOM | Assignee: | Nobody <webkit-unassigned> | ||||||||||
| Status: | RESOLVED FIXED | ||||||||||||
| Severity: | Minor | CC: | cdumez, ddkilzer, rwlbuis | ||||||||||
| Priority: | P1 | ||||||||||||
| Version: | 420+ | ||||||||||||
| Hardware: | Mac | ||||||||||||
| OS: | OS X 10.4 | ||||||||||||
| Attachments: |
|
||||||||||||
|
Description
Darin Adler
2006-06-26 09:16:09 PDT
Created attachment 9060 [details]
First attempt
The patch inits pageX/pageY ok, and adds testing code to window-xy-properties.html to
verify it. I am not sure whether the other FIXME are part of this bug, I guess they are simple
to fix and either they should be protected or have setters.
Cheers,
Rob.
Comment on attachment 9060 [details]
First attempt
This looks wrong to me. The initMouseEvent function is passing clientX and clientY to a function that takes parameters named pageX and pageY.
The parameters to initMouseEvent are not the page x and y, but rather the client x and y. You can check this with other browsers, but I'm pretty sure this is true.
Created attachment 9066 [details]
Improved patch
Hi Darin,
You are right, this should fix it.
Cheers,
Rob.
Comment on attachment 9066 [details]
Improved patch
I think it's pretty ugly to have two different initCoordinates functions, but I think this will work right.
Ultimately we could change this all so that pageX/Y is always computed from clientX/Y. The only reason it's not like that today is Macintosh-specific stuff: the "viewport" coordinates for the web view are actually window coordinates on OS X so they can't be used directly for clientX/Y. Otherwise, we'd go change all the callers to pass in clientX/Y and the code would all match. It would be great to straighten that out some day.
Patch needs to include the patch to the test results too, though.
r=me, but review- because we need a patch that includes fast/dom/window-xy-properties-expected.txt.
Created attachment 9067 [details]
Now with expected results
Yup, it was not properly generated, now it is included in the patch.
Cheers,
Rob.
Comment on attachment 9067 [details]
Now with expected results
The values here aren't right for offsetX/Y. The reason for that is that they can't be correct until you dispatch the event to an object. We really should be testing the values of these after dispatching too.
r=me, we can deal with that later
Committed revision 15076. Created attachment 9072 [details]
Improved testcase
The improved testcase now dispatches the event, so offsetX and offsetY are adjusted, the
test verifies it. Please let me know whether I need to make Changelog entries for the new patch to land.
Cheers,
Rob.
Reopening bug per Comment #9. Rob, please file a new bug, even for updated tests. The best thing to do would be to set the blocks/depends on flag to reference the original bug. See Bug 8707, Comment #27. Comment on attachment 9072 [details]
Improved testcase
better -- r=me
Committed revision 15078. (In reply to comment #10) > Rob, please file a new bug, even for updated tests. The best thing to do would > be to set the blocks/depends on flag to reference the original bug. Yes, Dave's right that we don't want to reuse a bug. The reason is that the bug should represent the original problem and be reopened if it wasn't fixed. We don't want to reopen a bug to refine a fix, but rather just if the fix didn't work for some reason. This helps us be clearer on the "verification" step and understanding what is working and what isn't. Hi Darin, Dave, (In reply to comment #13) > (In reply to comment #10) > > Rob, please file a new bug, even for updated tests. The best thing to do would > > be to set the blocks/depends on flag to reference the original bug. > > Yes, Dave's right that we don't want to reuse a bug. > > The reason is that the bug should represent the original problem and be > reopened if it wasn't fixed. We don't want to reopen a bug to refine a fix, but > rather just if the fix didn't work for some reason. This helps us be clearer on > the "verification" step and understanding what is working and what isn't. Ok, will try to remember with future bug reports :) Cheers, Rob. Mass moving XML DOM bugs to the "DOM" Component. |