Bug 9605 - REGRESSION: pageX and pageY are both 0 for events created with initMouseEvent
: REGRESSION: pageX and pageY are both 0 for events created with initMouseEvent
Status: RESOLVED FIXED
: WebKit
XML DOM
: 420+
: Macintosh Mac OS X 10.4
: P1 Minor
Assigned To:
:
:
:
:
  Show dependency treegraph
 
Reported: 2006-06-26 09:16 PST by
Modified: 2006-06-28 12:34 PST (History)


Attachments
First attempt (4.78 KB, patch)
2006-06-27 06:09 PST, Rob Buis
darin: review-
Review Patch | Details | Formatted Diff | Diff
Improved patch (4.74 KB, patch)
2006-06-27 13:34 PST, Rob Buis
darin: review-
Review Patch | Details | Formatted Diff | Diff
Now with expected results (5.31 KB, patch)
2006-06-27 14:04 PST, Rob Buis
darin: review+
Review Patch | Details | Formatted Diff | Diff
Improved testcase (2.44 KB, patch)
2006-06-28 00:08 PST, Rob Buis
darin: review+
Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2006-06-26 09:16:09 PST
The fix for bug 8707 introduced this problem.
------- Comment #1 From 2006-06-26 09:16:26 PST -------
Bug 8707.
------- Comment #2 From 2006-06-27 06:09:59 PST -------
Created an attachment (id=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 #3 From 2006-06-27 09:29:50 PST -------
(From update of attachment 9060 [details])
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.
------- Comment #4 From 2006-06-27 13:34:59 PST -------
Created an attachment (id=9066) [details]
Improved patch

Hi Darin,

You are right, this should fix it.
Cheers,

Rob.
------- Comment #5 From 2006-06-27 13:43:13 PST -------
(From update of attachment 9066 [details])
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.
------- Comment #6 From 2006-06-27 14:04:13 PST -------
Created an attachment (id=9067) [details]
Now with expected results

Yup, it was not properly generated, now it is included in the patch.
Cheers,
Rob.
------- Comment #7 From 2006-06-27 14:58:50 PST -------
(From update of attachment 9067 [details])
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
------- Comment #8 From 2006-06-27 20:47:01 PST -------
Committed revision 15076.
------- Comment #9 From 2006-06-28 00:08:03 PST -------
Created an attachment (id=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.
------- Comment #10 From 2006-06-28 04:08:49 PST -------
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 #11 From 2006-06-28 07:30:42 PST -------
(From update of attachment 9072 [details])
better -- r=me
------- Comment #12 From 2006-06-28 07:35:17 PST -------
Committed revision 15078.
------- Comment #13 From 2006-06-28 07:36:58 PST -------
(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.
------- Comment #14 From 2006-06-28 12:34:18 PST -------
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.