need some easy way to get the screen coordinates of a xul element

RESOLVED FIXED in mozilla0.9.1

Status

()

defect
RESOLVED FIXED
19 years ago
19 years ago

People

(Reporter: hewitt, Assigned: hewitt)

Tracking

Trunk
mozilla0.9.1
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

I really need a way to get the screen coordinates of a xul element.  The reason
I need it is because I just discovered that when you capture a mouse event
inside of a popup, the event.clientX/Y properties will actually like and give
you screen coordinates instead of client coordinates.  I need to turn them back
into clinet coordinates, and I need to be able to say something like -
box.boxObject.screenY

I thought there would be an easy way to do this, but I can't find one.  All I
could find was nsIWidget::WidgetToScreen, but since not all xul elements have a
widget this is not very useful.  Fortunately, this is useful for me, as I only
need to know the coordinates of a popup for this application.  I hacked screenX
and screenY into nsIBoxObject in my tree, but maybe that's not the best way to
do this?
->hyatt
Assignee: trudelle → hyatt
Blocks: 43189
Posted patch patch to fixSplinter Review
taking
Assignee: hyatt → hewitt
Target Milestone: --- → mozilla0.9.1
The patch leaks all over:

+    rv = frame->GetView(presContext, &view);
+    if (view) {
+      rv = view->GetWidget(widget);
+      if (widget)
+        break;
+    }

Use an nsCOMPtr, or NS_RELEASE after calling view->GetWidget and before the if
(widget) break; -- but really, use nsCOMPtr.

For widget too, which is leaked if found.  The control flow here:

+  if (widget) {
+    // add the widget's screen coordinates to the offset we've counted
+    nsRect oldBox(0,0,0,0);
+    widget->WidgetToScreen(oldBox, aRect);
+    aRect.x += offsetX;
+    aRect.y += offsetY;
+  } else
+    return NS_ERROR_FAILURE;
+
+  return NS_OK;

is capricious: why put the return NS_ERROR_FAILURE in the else clause, but the
final return not in the then clause?  Of course, then you'd have an instance of
the else-after-return non-sequitur.  Best to cast out failure cases with early
returns:

+  if (!widget)
+    return NS_ERROR_FAILURE;
+
+  // add the widget's screen coordinates to the offset we've counted
+  nsRect oldBox(0,0,0,0);
+  widget->WidgetToScreen(oldBox, aRect);
+  aRect.x += offsetX;
+  aRect.y += offsetY;
+  return NS_OK;

supposing, again, that widget is nsCOMPtr<nsIWidget>.

Same goes for doc, except that the use of a reference to a pointer in
nsIContent::GetDocument requires special handling:

+  nsIDocument* doc;
+  mContent->GetDocument(doc);

should be

+  nsCOMPtr<nsIDocument> doc;
+  mContent->GetDocument(*getter_AddRefs(doc));

/be
Posted patch patch, take 2Splinter Review
Still leaking the view if found:

+    nsIView* view;
+    rv = frame->GetView(presContext, &view);
+    if (view) {
+      rv = view->GetWidget(*getter_AddRefs(widget));
+      if (widget)
+        break;
+    }

That was my first comment ("but really, use nsCOMPtr").

Can the content have lost its document, via DOM manipulations, yet still have a
script ask for its screenX or Y?  If so, you should null-check doc right after
getting it.

/be
I am confused by your suggestion to use nsCOMPtr on nsIView, because it seems
that views are not ref counted... or at least, that's what the compiler told me
when I tried to do an nsCOMPtr<nsIView>.  Am I missing something here?

Status: NEW → ASSIGNED
Sorry, I'm wrong: views, like frames, are not ref-counted.  Evil, but I forgot
views shared that badness with frames.

Is the doc null check necessary?  Waterson would know (hyatt too, I bet).

/be
Themes Triage Team Marking nsbeta1+
Keywords: nsbeta1+
Judging by the implementation of GetOffsetRect, right above GetScreenRect, I'd
say I need to be null checkin doc and presShell.  I modified the patch so that
it looks similar to GetOffsetRect.
Posted patch patch, take 3Splinter Review
[s]r=hyatt
fixed
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.