312936 - Add methods to calculate the location of frame

Status: RESOLVED INVALID

(Core :: Layout, defect)

Description

[Eli Friedman]

Various places need to know where a frame is relative to the document or the
screen.  I propose adding two methods to nsIFrame:

/**
 * Calculate the location of top left corner of the frame
 * relative to the top left corner of the document.
 *
 * @return point in app units
 */
nsPoint ClientOrigin();

/**
 * Calculate the location of top left corner of the frame
 * relative to the top left corner of the screen.
 *
 * @return point in app units
 */
nsPoint ScreenOrigin();

The implementation of ClientOrigin I think will just be "return
GetOffsetTo(GetPresContext()->PresShell()->FrameManager()->GetRootFrame());"

The implementation of ScreenOrigin will take ClientOrigin, convert that into the
nearest view's coordinates, convert it into the nearest widget's coordinates,
then convert that to screen coordinates.

Comments?

Comment 1

[Eli Friedman]

I just remembered that unmarked methods can't be called cross-module, so make
those virtual or something like that, I guess.

Actually, I'm kind of curious why layout doesn't use NS_EXPORT intstead of
making methods that need to be called across modules virtual.  Is there a speed
cost?  Or is there some other issue?

Comment 2

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

Your methods sound reasonable.

I'm actually not 100% sure why we use virtual instead of NS_EXPORT ... I think
perhaps NS_EXPORT can't be applied to methods on all platforms?

dbaron,bz: the context is that code outside gklayout (accessibility, inspector,
etc) needs frame position information and currently has to go through views to
get it, which sucks, so some new frame APIs are needed.

Another approach might be to create a service, let's call it nsIFrameInspector,
and give it methods like

  nsPoint GetClientOrigin(nsIFrame* f);
  nsPoint GetScreenOrigin(nsIFrame* f);

This would have the advantage of a) being potentially freezable b) possibly even
becoming scriptable c) not bloating frame class vtables.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Hmm...  Why don't we use NS_EXPORT indeed?  ccing some folks who might know
about it.

Note that using NS_EXPORT works fine for gfx (NS_GFX macro), but those are
extern "C" methods... if that matters.  Also works fine for XPCOM (NS_COM
macro), and nsVoidArray is not at all extern "C".

For the rest, if we can avoid increasing the vtable size, I'd say put this on
nsIFrame -- using a service would involve some sort of null-checks in the
service or something, most likely...  Even if we have to use virtual, I think
the simplicity of having this on nsIFrame is worth it -- the increase in vtable
size might be less than the extra code for getting that service at the various
callsites.

Comment 4

[Benjamin Smedberg]

You can't NS_EXPORT anything from layout, because layout is a component and
therefore nothing links against it.

Comment 5

[Eli Friedman]

Thanks for pointing out that layout is a component.  I suppose that restricts us
to making functions virtual, whether on nsIFrame or some new interface.  Now
back to the issue of the functions to be added.

1. I just discovered GetScreenRectExternal, which does exactly what my
"ScreenOrigin" would.

2. I just went through all the callers of GetViewExternal and GetOffsetFromView,
because those seem the most relevant to any functions we might add in this bug.
 My notes (skip to the end for the summary):

GetViewExternal Callers:
•	GetEditorContentWindow calls it as an unnecessary optimization
•	nsHTMLEditor::GetElementOrigin calls it to get the coordinates relative to the
nearest widget, although I don’t quite follow how it uses them; could possibly
use a “ViewportRect” function
•	nsAccessible::IsPartiallyVisible (see below)
•	inFlasher::RepaintElement calls it to repaint a frame; should we expose
nsIFrame::Invalidate() or something like that?
•	nsAccessibleWrap::get_accParent calls it in the process of getting a widget,
although I don’t follow what the function is doing with the views
•	nsAccessible::GetScreenOrigin (see below)
•	inLayoutUtils::GetScreenOrigin calls it in the process of finding the screen
coordinates of a frame; should use GetScreenRectExternal

GetOffsetFromView callers (outside of layout):
•	inLayoutUtils::GetClientOrigin gets coordinates relative to the nearest widget
so that the frame’s outline can be painted; there really ought to be a better
way to do this.
•	nsAccessible::IsPartiallyVisible calls it in finding whether a frame is
visible on the screen
•	nsTypeAheadFind::IsRangeVisible (both the extensions and toolkit versions)
call it in finding whether a frame is visible; some sort of “IsFrameVisible”
function would help, although I’m not exactly sure what it would look like
•	makeRectRelativeToGlobalView calls it for finding frame position relative to
other frames; would benefit from a “ViewportRect” function
•	nsAccessible::GetScreenOrigin calls it to find the location of a frame on the
screen; should use GetScreenRectExternal
•	nsDragService::InvokeDragSession and 
nsDragService::ComputeGlobalRectFromFrame use it to find the location of a frame
on the screen; should use GetScreenRectExternal
•	GetScreenOrigin (in nsFormFillController.cpp) uses it to find the location of
a frame on the screen; should use GetScreenRectExternal

The overall points:
Some cleanup is needed to make various places use GetScreenRectExternal.
“ViewportRect” and “IsFrameVisible” functions would be useful in multiple
places.  Inspector needs an alternate way to figure out where to paint and to
repaint what it paints over.  I don’t understand what
nsAccessibleWrap::get_accParent is doing.

Comment 6

[Boris Zbarsky [:bzbarsky]]

aaronlev, see end of comment 5.  Can you help?

Comment 7

[Aaron Leventhal]

get_accParent() needs to know when it the current accessible object represents the root object for a native window (if it is, then we hand back an accessible for the native window as the parent).

There are times when 2 native windows are used together, one containing the scrollbars and the other only the client area with no scrollbars. When we hand back an accessible for the native window in this situation, we want the outer window which contains the scrollbars.

So the code that deals with views and widgets basically:
1) determines if we're at the root of a view with a widget
2) if we are determines whether there is a pair of native windows or a single one, and hands back the appropriate native window.

Which part are you trying to understand?

Comment 8

[Eli Friedman]

I think that makes sense.  Thanks.  I guess eventually accessibility's going to be responsible for creating its own widgets to manage the accessibility objects.

roc, I'm not exactly sure in what order you were think of getting rid of the widgets and the views.  I'm thinking it might be easier to deal with the widgets first.

Comment 9

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

Yeah, probably, but I'm not sure.

Comment 10

[Eli Friedman]

I don't think there's really anything useful in this bug.