Bug 9605 – REGRESSION: pageX and pageY are both 0 for events created with initMouseEvent

• Status: RESOLVED
• Resolution: FIXED
• Component: DOM

[Darin Adler]

The fix for bug 8707 introduced this problem.

[Darin Adler]

Bug 8707.

[Rob Buis]

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.

[Darin Adler]

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.

[Rob Buis]

Created attachment 9066 [details]
Improved patch

Hi Darin,

You are right, this should fix it.
Cheers,

Rob.

[Darin Adler]

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.

[Rob Buis]

Created attachment 9067 [details]
Now with expected results

Yup, it was not properly generated, now it is included in the patch.
Cheers,
Rob.

[Darin Adler]

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

[Darin Adler]

Committed revision 15076.

[Rob Buis]

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.

[David Kilzer (:ddkilzer)]

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.

[Darin Adler]

Comment on attachment 9072 [details]
Improved testcase

better -- r=me

[Darin Adler]

Committed revision 15078.

[Darin Adler]

(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.

[Rob Buis]

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.

[Lucas Forschler]

Mass moving XML DOM bugs to the "DOM" Component.