Closed Bug 845595 Opened 12 years ago Closed 12 years ago

Fix up nsIDOMWindowUtils::selectAtPoint coordinate reference

Categories

(Core :: Widget, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla22

People

(Reporter: mstange, Assigned: mstange)

References

Details

Attachments

(1 file)

Attached patch fix (deleted) — Splinter Review
The patch in bug 625989 causes test_selectAtPoint.html to fail. nsDOMWindowUtils::SelectAtPoint currently treats aX and aY as coordinates relative to the widget. Bug 625989 moves the widget up, and the client area down (in relation to the widget), so the client coordinates that test_selectAtPoint.html uses no longer hit the right elements. The documentation for nsIDOMWindowUtils::selectAtPoint says "Select content at a client point [...]", so I think it's the implementation of nsDOMWindowUtils::SelectAtPoint that needs to be fixed. I think the coordinates should be relative to the current document. The second part of test_selectAtPoint.html calls selectAtPoint on an iframe, or rather on the iframe's document's DOMWindowUtils instance. If we make the coordinates relative to the DWU's document, then the test no longer needs to add the iframe position to the coordinates. Jimm, do you think this suggestion makes sense?
Attachment #718763 - Flags: review?(jmathies)
Yes. I totally ran into this behavior the other day and was like huh? When you call selectAtPoint on the sub-frame, it should accept coords relative to the frame's origin.
Comment on attachment 718763 [details] [diff] [review] fix Ran this through a set of metro selection tests w/out compensating for the sub-frame behavior, everything works great. Thanks!
Attachment #718763 - Flags: review?(jmathies) → review+
Good. (In reply to Jim Mathies [:jimm] from comment #2) > a set of metro selection tests w/out compensating for the > sub-frame behavior Does this mean that you removed code that compensated for the sub-frame behavior anywhere? Do I need to include those changes in the patch or can I check it in as is? Pushed to try, just to be safe: https://tbpl.mozilla.org/?tree=Try&rev=d15df41e03eb
(In reply to Markus Stange from comment #3) > Good. > > (In reply to Jim Mathies [:jimm] from comment #2) > > a set of metro selection tests w/out compensating for the > > sub-frame behavior > > Does this mean that you removed code that compensated for the sub-frame > behavior anywhere? Do I need to include those changes in the patch or can I > check it in as is? > > Pushed to try, just to be safe: > https://tbpl.mozilla.org/?tree=Try&rev=d15df41e03eb Yes, land away. I removed code from test that haven't landed on mc yet.
Assignee: nobody → mstange
Status: NEW → ASSIGNED
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
Should we update the header comments here? The header states "Select content at a client point based on a selection behavior..." That doesn't seem to jibe with the behavior of the call which expects document relative coordinates.
I actually don't know what "based on a selection behavior" means. Are coordinates in selection APIs usually not document relative? But sure, if you think the comment can be clarified, go ahead.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: