306202 - onpopupshowing event.clientX is always 0

Status: RESOLVED WORKSFORME

(Core :: XUL, defect)

Description

[Joey Minta]

Regression from bug 296036.  Prior to the checkin there, onpopupshowing events
had clientX and clientY values that generally were not 0.  

event.screenX and event.screenY are also 0 for onpopupshowing events, but this
problem dates back to at least FF 1.0.x

Comment 1

[Joey Minta]

Attachment: onpopupshowing testcase

This testcase shows event.clientX/Y as well as event.screenX/Y since I wasn't
sure if the problems were related or not.  clientX/Y is the regression.

Comment 2

[Eli Friedman]

Attachment: Possible Patch

This is one possibility, although it definitely abuses how Mozilla uses
widgets.  I'm not sure about the proper way to do this; suggestions?

Comment 3

[Eli Friedman]

Attachment: Patch

Comment 4

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

What do the XUL users of onpopushowing expect in clientX/clientY?

Comment 5

[[e-mail]]

I didn't realize popupshowing was a mouse event... that explains the bug reports
by people expecting to be able to read the modifiers.

Note that those coordinates are only desired coordinates (as passed to
ShowPopup) and the actual location is determined later by DoLayout. In
particular AdjustClientXYForNestedDocuments ensures that they are fairly bogus.

Comment 6

[gekacheka]

MouseEvent.clientX and MouseEvent.clientY are mouse coordinates in the
document where the mouse event occurred (hover, drag, click, etc).
They are used to find the object from which to fill the popup, or on
which to act, in the document under the mouse.

In contrast to onclick and related handlers (there is no onhover
event) which are placed on the target elements of mouse action, an
onpopupshowing handler is placed on the shared popup element.  The
popup was not the target of the mouse event, but is usually opened
by a mouse event.  

So the question seems to be whether the onpopupshowing event should
include mouse information from the mouse action that triggered it.

Setting the onpopupshowing event mouse fields to the information from
the mouse event that triggered the popup (as before) is useful for
handlers that fill the popup based on the location of the mouse event
(using boxObject.x, .y or nsITreeBoxObject.getCellAt, .getRowAt).
Omitting this information is less useful.  If omitted it would be
difficult to workaround (in the case of hovering, have to add target
handlers to capture and store the coordinates somewhere ad hoc; in
other cases have to move the handler from the popup to each target).

Is there any downside to defining the onpopupshowing event mouse
attributes to hold information from the mouse action that triggered
the popup (as before for clientX, clientY)?


References:

onpopupshowing "is usually used to dynamically set the contents" of the popup.
http://www.xulplanet.com/references/elemref/ref_EventHandlers.html#attr_onpopupshowing

DOM MouseEvent.clientX, .clientY
http://www.w3.org/TR/DOM-Level-3-Events/events.html#Events-MouseEvent-clientX

XUL tutorial: mouse x y
http://www.xulplanet.com/tutorials/xulqa/q_mousexy.html 

XULElement.boxObject.x, .y
http://www.xulplanet.com/references/elemref/ref_XULElement.html#prop_boxObject.x

nsITreeBoxObject.getCellAt
http://www.xulplanet.com/references/xpcomref/ifaces/nsITreeBoxObject.html#method_getCellAt

nsITreeBoxObject.getRowAt
http://www.xulplanet.com/references/xpcomref/ifaces/nsITreeBoxObject.html#method_getRowAt

Comment 7

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

Maybe a more sensible API would have onpopupshowing not being a mouse event, but
instead holding a reference to the event that caused it?

Comment 8

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

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

minusing until we resolve what the API should be

Comment 9

[[e-mail]]

(In reply to comment #7)
>Maybe a more sensible API would have onpopupshowing not being a mouse event, but
>instead holding a reference to the event that caused it?
Well, I can think of five ways in which a popup could get shown:
a) clicking on a XUL element with a "popup" attribute
b) right-clicking on a XUL element with a context menu
c) focussing a XUL element with a context menu and pressing the menu key
d) calling showPopup explicitly from script
e) hovering an element with a tooltip (which is a popup)
It's nontrivial to get the causing event for cases d) and e) ;-)

Comment 10

[[e-mail]]

And case c) has no coordinates either, of course.

Comment 11

[Eli Friedman]

I'm not very knowledgeable about XUL, so it's not really appropriate for me to hold on to this bug.  I can answer any event-related questions, though.

Comment 12

[Martijn Wargers (dead)]

It seems to me this bug is resolved somehow.
Now event.client and event.screen properties show the right number with the testcase.

Comment 13

[Nickolay_Ponomarev]

Attachment: a working testcase

without the typo.

Comment 14

[Nils Maier [:nmaier]]

Attachment: testcase using a tooltip + popupshowing

The client? properties are set in 1.8 but not in 1.9 (rv:1.9b3pre Gecko/2007122017)
We (used to) use this to determine the currently hovered <tree> row, which now won't work (without an ugly mousemove workaround).

Affected extension: DownThemAll! (the Manager window).
Example: http://bugs.code.downthemall.net/trac/browser/branches/0.9.x/chrome/content/dta/down.js#L3649

Comment 15

[Nils Maier [:nmaier]]

The mouseover workaround is plain ugly.
There should be some kind of API to get the current mouse position within the client box, no matter what it looks like, hence reopening.

Comment 17

[Neil Deakin]

There are four means of opening a popup:

1. Clicking on a menu or menu-like element.
2. Clicking on an element with a popup attribute or context attribute, or using the key equivalents.
3. Hovering a tooltip
4. Opening the popup with the api

#1 is not and has never been implemented. The event properties are always 0.
#2 has always been implemented, except for a period when there was a regression from the popup reworking and fixed by bug 387659. This is what the testcases in this bug are checking.
#3 is bug 413268.
#4 doesn't make any sense as there is no mouse event.

The bug as filed is 'worksforme'. There should be a separate bug filed for #1 if someone wants it implemented.