268576 - [FIXr]Method to get screen coordinates on nsIFrame

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Aaron Leventhal]

Spun off from bug 175568.

Here's the list of places that Boris mentioned all use some method of getting
the screen coordinates from a frame:
  nsMenuPopupFrame::GetScreenPosition
  nsSVGSVGElement::GetScreenPosition
  nsBoxObject::GetScreenRect
  nsAccessibleText::GetCharacterExtents (this has the COORD_TYPE_WINDOW
    weirdness, though).
  Some Inspector code that's unused and removed in my tree...

Comment 1

[Boris Zbarsky [:bzbarsky]]

Note: bug 175568 will add another consumer of this sort of thing.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Attachment: Patch

Comment 3

[Boris Zbarsky [:bzbarsky]]

Note that for views I did position, not rect, because the bounds topleft may not
match the origins and callers seemed to want the origin.

Comment 4

[Aaron Leventhal]

Doesn't look like we can use this for context menu stuff in
nsEventListenerManager after all, since that's not in screen coordinates. Too bad.

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 166147 [details] [diff] [review]
Patch

+      // XXXbz there needs to be a better way to do this scaling
+      nsIntRect ourPxRect(NSTwipsToIntPixels(ourRect.x, t2p),
+			   NSTwipsToIntPixels(ourRect.y, t2p),
+			   NSTwipsToIntPixels(ourRect.width, t2p),
+			   NSTwipsToIntPixels(ourRect.height, t2p));

There is: nsRect::ScaleRoundOut. With that, r+sr=roc

Comment 6

[Boris Zbarsky [:bzbarsky]]

But that rounds out instead of rounding to the nearest int (which is what the
code in question does, I think).  Do we care?

Comment 7

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I don't think so. It really depends on the caller, but I bet that rounding out
is always no less correct than rounding to the nearest int.

Comment 8

[Boris Zbarsky [:bzbarsky]]

Sounds good.

Comment 9

[Boris Zbarsky [:bzbarsky]]

Note to self:

> +    frameScreenRect.x += NS_TwipsToIntPixels(beforeWidth. t2p);

Should have a comma, not a dot, after beforeWidth (this is in the accessibility
code).

Comment 10

[Boris Zbarsky [:bzbarsky]]

Attachment: Updated to comments

Comment 11

[Boris Zbarsky [:bzbarsky]]

Fixed on trunk.

Comment 12

[Boris Zbarsky [:bzbarsky]]

This caused bug 271702 (because that code mis-treated screenY) and bug 271891
(because this patch had an error in it).

Comment 13

[Martijn Wargers (dead)]

Could this have caused bug 273226?