133426 - can't get coordinates from popup events

Status: VERIFIED FIXED

(Core :: XUL, defect, P2)

Description

[Joe Hewitt (gone)]

A long time ago I checked in something to allow event.clientX/Y properties to
work for popup events ("onpopupshowing" etc...).  This caused a crash and had to
be backed out (see bug 120237).

I need to fix this again so that tree (outliner) tooltips can be supported.

Comment 1

[Joe Hewitt (gone)]

nominating, AIM needs this in order to restore the buddy tooltips feature

Comment 2

[Joe Hewitt (gone)]

Attachment: patch

Comment 3

[Johnny Stenback (:jst)]

Comment on attachment 76153 [details] [diff] [review]
patch

>       mEvent = PR_NEWZAP(nsMutationEvent);
>       mEvent->eventStructType = NS_MUTATION_EVENT;
>     }
>+    else if (eventType.EqualsIgnoreCase("PopupEvents")) {
>+      mEvent = PR_NEWZAP(nsEvent);
>+      mEvent->eventStructType = NS_POPUP_EVENT;
>+    }
[...]
> NS_METHOD nsDOMEvent::GetClientX(PRInt32* aClientX)
> {
>   if (!mEvent || 
>-       (mEvent->eventStructType != NS_MOUSE_EVENT && mEvent->eventStructType != NS_DRAGDROP_EVENT) ||
>+       (mEvent->eventStructType != NS_MOUSE_EVENT &&
>+        mEvent->eventStructType != NS_POPUP_EVENT &&
>+        mEvent->eventStructType != NS_DRAGDROP_EVENT) ||
>       !mPresContext) {
>     *aClientX = 0;
>     return NS_OK;

This will cause the exact same problem that caused the original fix for this to
be backed out. nsDOMEvent::GetScreen[X|Y]() casts mEvent to nsGUIEvent, but
you're creating nsEvent objects for "PopupEvents" events, and not nsGUIEvent
objects. That needs fixin', make the nsDOMEvent constructor create nsGUIEvent
objects for "PopupEvents" events.

>-nsXULTooltipListener::GetTargetOutlinerBoxObject(nsIOutlinerBoxObject** aBoxObject)
>+nsXULTooltipListener::GetSourceOutlinerBoxObject(nsIOutlinerBoxObject** aBoxObject)
> {

This method needs to default *aBoxObject to nsnull.

[...]
>+  if (mIsSourceOutliner && mSourceNode) {
>+    nsCOMPtr<nsIContent> outlinerParent;
>+    mSourceNode->GetParent(*getter_AddRefs(outlinerParent));
>+    if (outlinerParent) {
>+      nsCOMPtr<nsIDOMXULElement> xulEl(do_QueryInterface(outlinerParent));

The above if (outlinerParent) check is unnecessary, do_QueryInterface() is null
safe.

>+      if (xulEl) {
>+        nsCOMPtr<nsIBoxObject> bx;
>+        xulEl->GetBoxObject(getter_AddRefs(bx));
>+        nsCOMPtr<nsIOutlinerBoxObject> obx(do_QueryInterface(bx));
>+        if (obx) {
>+          *aBoxObject = obx;
>+          NS_ADDREF(*aBoxObject);
>+          return NS_OK;
>+        }
>       }
>-    }
>+  }
>   }
>   return NS_ERROR_FAILURE;
> }

sr=jst if you fix that.

Comment 4

[Joe Hewitt (gone)]

Attachment: new patch

Comment 5

[Joe Hewitt (gone)]

Comment on attachment 76337 [details] [diff] [review]
new patch

carrying over sr=jst

Comment 6

[John Gaunt (redfive)]

Comment on attachment 76337 [details] [diff] [review]
new patch

r=jgaunt

Comment 7

[Asa Dotzler [:asa]]

Comment on attachment 76337 [details] [diff] [review]
new patch

a=asa (on behalf of drivers) for checkin to the 1.0 trunk

Comment 8

[Joe Hewitt (gone)]

feexed

Comment 9

[shrirang khanzode]

v(stamp)