Closed
Bug 845595
Opened 12 years ago
Closed 12 years ago
Fix up nsIDOMWindowUtils::selectAtPoint coordinate reference
Categories
(Core :: Widget, defect)
Core
Widget
Tracking
()
RESOLVED
FIXED
mozilla22
People
(Reporter: mstange, Assigned: mstange)
References
Details
Attachments
(1 file)
(deleted),
patch
|
jimm
:
review+
|
Details | Diff | 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)
Comment 1•12 years ago
|
||
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 2•12 years ago
|
||
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+
Assignee | ||
Comment 3•12 years ago
|
||
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
Comment 4•12 years ago
|
||
(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 | ||
Comment 5•12 years ago
|
||
Oh, I see, thanks.
https://hg.mozilla.org/integration/mozilla-inbound/rev/f223a4961926
Assignee | ||
Updated•12 years ago
|
Assignee: nobody → mstange
Status: NEW → ASSIGNED
Comment 6•12 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
Comment 7•12 years ago
|
||
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.
Assignee | ||
Comment 8•12 years ago
|
||
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.
Description
•