User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.8.1.4) Gecko/20070508 Iceweasel/2.0.0.4 (Debian-2.0.0.4-0etch1) Build Identifier: XULRunner 1.9a7pre - 2007070319 In version 1.8.0.12 I used treeBoxObject.getCellAt to get the row+column of the item that was right-click (contextmenu). In version 1.9a7pre treeBoxObject no longer exists and there appears to be no other way to get the row+column information. xul-code <popupset> <popup id="treeActionMenu" onpopupshowing="changeTreePopup(event);"> </popup> </popupset> <tree flex="1" id="treecluster"> <treecols> <treecol id="resources" label="Resources" primary="true" flex="1" /> </treecols> <treechildren context="treeActionMenu" id="clusterTree"> </treechildren> </tree> js-code: function changeTreePopup(event) { var tree = event.rangeParent; if(!tree) return; var row = new Object(); var col = new Object(); var childElement = new Object(); tree.treeBoxObject.getCellAt(event.clientX, event.clientY, row, col, childElement); Reproducible: Always Steps to Reproduce: 1. 2. 3.

Are you sure? I see plenty of places in the tree where we're using this; e.g. http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/browser/components/places/content/sidebarUtils.js&rev=1.1&mark=41-44#39.

I've made a small test. It appears the event passed to the popup-handler is wrong. Not the treeBoxObject. In version 1.9a7pre, event.clientX and event.rangeParent is undefined in the popuphandler but it is in the mousemovehandler. (code below) xulrunner 1.8.0.12: M: treecluster 101 55 C: treecluster 103 57 1 [object TreeColumn] M: treecluster 101 55 xulrunner 1.9a7pre: M: treecluster 111 49 rangeParent undefined C: treecluster undefined undefined -1 null M: treecluster 111 49 tree.xul <?xml version="1.0"?> <?xml-stylesheet href="chrome://global/skin/" type="text/css"?> <window id="treebug" title="treebug" width="400" height="500" xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"> <script src="tree.js"/> <popupset> <popup id="treeActionMenu" onpopupshowing="changeTreePopup(event);"> <menuitem label="Hello world"/> </popup> </popupset> <tree flex="1" id="treecluster" onmousemove="updateMouseCoordinates(event);"> <treecols> <treecol id="resources" label="Resources" primary="true" flex="1" /> </treecols> <treechildren context="treeActionMenu" id="clusterTree"> <treeitem label="test 1"/> <treeitem label="test 2"/> </treechildren> </tree> </window> tree.js function updateMouseCoordinates(event) { var tree = event.rangeParent; println("M: " + tree.id + " " + event.clientX + " " + event.clientY); } function changeTreePopup(event) { var tree = event.rangeParent; if(!tree) { println("rangeParent undefined"); tree = event.explicitOriginalTarget.parentNode; } println("C: " + tree.id + " " + event.clientX + " " + event.clientY); var row = new Object(); var col = new Object(); var childElement = new Object(); tree.treeBoxObject.getCellAt(event.clientX, event.clientY, row, col, childElement); println(row.value + " " + col.value); } function println(s) { dump(s + "\n"); }

The issue here is that the x/y coordinates are not being set in the popupshowing event. This apparently happened for some popups before but not all, whereas now it isn't happening for any of them. Your example doesn't show how you open the popup though, so it's hard to tell.

The 2 files I gave contain everything needed to generate the problem. By setting 'context="treeActionMenu"' the popup is opened.

Comment on attachment 272499 [details] [diff] [review] use mouse events and get right coordinates in popupshowing event I have to admit I'm not up on our event stuff. In particular, I'd like eli to look at the refpoint/widget stuff (should we be using the same widget as the original event, e.g.?) and smaug to look at the general use of nsMouseEvent here.

Why are you adding a parameter to GetMouseLocation? It doesn't appear to be used in this patch. Your code to cache mCachedMousePoint is wrong; you need to translate into some fixed coordinate system. Not sure what widgets are conveniently available from SetMouseLocation; maybe relative to the root frame/widget of the root prescontext? You should make sure to add a test using overflow: auto or an XUL deck.

(In reply to comment #7) > Why are you adding a parameter to GetMouseLocation? It doesn't appear to be > used in this patch. > I'll remove that. > Your code to cache mCachedMousePoint is wrong; you need to translate into some > fixed coordinate system. Not sure what widgets are conveniently available from > SetMouseLocation; maybe relative to the root frame/widget of the root > prescontext? > Would it be sufficient to use GetEventCoordinatesRelativeTo to get the coordinates relative to the root frame of the document containing the popup node? This is the widget used when the cached point is used again. This seems to work for the case inside a deck or iframe.

(In reply to comment #8) > Would it be sufficient to use GetEventCoordinatesRelativeTo to get the > coordinates relative to the root frame of the document containing the popup > node? This is the widget used when the cached point is used again. > > This seems to work for the case inside a deck or iframe. Yes, that should be fine. I wasn't sure if you had an easy way to get to it from there.

Comment on attachment 272546 [details] [diff] [review] get right widget coordinates Looks fine

Comment on attachment 272546 [details] [diff] [review] get right widget coordinates Should be AppUnitsToDevPixels, not AppUnitsToIntCSSPixels. Otherwise looks fine.