Closed
Bug 287357
Opened 19 years ago
Closed 9 years ago
showPopup() could display a popup at the wrong position
Categories
(Core :: XUL, defect)
Core
XUL
Tracking
()
RESOLVED
WONTFIX
People
(Reporter: antonus, Unassigned)
References
(Depends on 1 open bug, )
Details
Attachments
(1 file)
1.15 KB,
application/vnd.mozilla.xul+xml
|
Details |
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7.6) Gecko/20050223 Firefox/1.0.1 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7.6) Gecko/20050223 Firefox/1.0.1 I want to show a popup at the mouse pointer position with the function showPopup. the position is calculated with (event.clientX+document.documentElement.boxObject.screenX, event.clientY+document.documentElement.boxObject.screenY). After the bug as occured, it act like if event.clientX = event.clientX+document.documentElement.boxObject.screenX, the same for clientY and screenY. so, the real (buggy) position becomes (event.clientX+document.documentElement.boxObject.screenX+document.documentElement.boxObject.screenX, event.clientY+document.documentElement.boxObject.screenY+document.documentElement.boxObject.screenY) Reproducible: Always Steps to Reproduce: 1. double click : the popup is correct 2. right click, the context popup is correct 3. double click again : the popup is incorrect Actual Results: the popup is not at the mouse pointer Expected Results: the popup is at the mouse pointer
Comment 1•19 years ago
|
||
Actually, the first one is in the middle of the screen for me, too... But did you bring this up because a page isn't working because of this? Or are you trying to create a page? Either way, it's possible that Firefox doesn't support it... Try the page in IE and let us know.
Comment 2•19 years ago
|
||
Alan, this bug is about popups in XUL. IE is not relevant here. Neil, roc, any idea what's going on here? Antoine, is this an issue with the coords the event reports, or with the showPopup call itself?
Reporter | ||
Comment 3•19 years ago
|
||
(In reply to comment #2) > Antoine, is this an issue with the coords the event reports, or with the > showPopup call itself? I added the calculated coords in the test page when ondblclick occurs. In both case the the coords are the same, so it is the showPopup that is broken. The first time, the placement is relative to the screen, the second time the placement is/seems relative to the document.
Comment 4•19 years ago
|
||
OK, this is ridiculous. First problem: the popup box object api doesn't document whether the coords are in the client coord space or in screen coord space. The code assumes the former. Second problem: The code tries to handle coordinate transformations caused by subdocuments via nsMenuPopupFrame::AdjustClientXYForNestedDocuments. This method doesn't actually take a node or anything (which it's caller, nsMenuPopupFrame::SyncViewWithFrame, doesn't have anyway). Instead, it tries to determine the "target" document as follows: // Find the widget associated with the target's document. // For tooltips, we check the document's tooltipNode (which is set by // nsXULTooltipListener). For regular popups, use popupNode (set by // nsXULPopupListener). I'm not sure why this code thinks that getting the document of the node set as the popupNode on the document that's the "popup" document should give the "target" document... but there it is. Apparently, calling showPopup() on a <popup> doesn't set the popupNode. Opening the context menu does, and doesn't unset it. So the second time we showPopup() we get the context menu's popup as the popupNode and end up with coords relative to the document instead of relative to screen (because we have a document to be relative to, all of a sudden). Neil, any idea what coords we _should_ be working in here, to start with? Given that, what's the deal with popupNode? And in any case, is all this code really needed? ;)
Status: UNCONFIRMED → NEW
Component: Layout → XP Toolkit/Widgets: XUL
Ever confirmed: true
QA Contact: layout
Comment 5•19 years ago
|
||
(In reply to comment #4) >Neil, any idea what coords we _should_ be working in here, to start with? Should? It appears to want event coordinates. At least, that's most convenient to the popup and context menu listeners. >Given that, what's the deal with popupNode? tooltipNode is set by a XUL element's tooltip listener whenever it decides it needs to show a tooltip. popupNode is supposed to be set by the context menu code, although for some reason the tooltip code manages to set it as well, for reasons that I haven't managed to determine, but it caused bug 102551.
Comment 6•19 years ago
|
||
> It appears to want event coordinates. OK. We can go with that. But event coordinates in which coordinate system, in the case of subframes, which is the case this broken code seems to be trying to address? > popupNode is supposed to be set by the context menu code The problem is that popupNode is relied on for all popups, not just the context menu code. And the context menu code never _un_sets it. So once you put up a context menu, the popupNode is always the context menu's popupNode, no matter what popup you're actually opening. I suppose we could change popup.xml to set the popupNode. Thing is, what would that break? And finally, what exact node is the popupNode supposed to be? The DOM node of the popup? Or the DOM node the popup is opening for? Or put another way, I'd really like to move away from the current document-adjusting code... I can't figure out why that code is working at all, or what objects it's operating on and why.
Comment 7•19 years ago
|
||
(In reply to comment #6) >>It appears to want event coordinates. >But event coordinates in which coordinate system In the system used by an event that bubbled up to showPopup's srcContent.
Comment 8•19 years ago
|
||
OK. So what we really want to do is to convert from the "document" coordinates of srcContent to the "document" coordinates of whatever document the mContent of the popup frame lives in, right?
Comment 9•19 years ago
|
||
The popup or tooltip should be in the same document as the srcContent because it is located by id (or is document body anonymous content for tooltiptext).
Comment 10•19 years ago
|
||
Ok... So is the popupNode allowed to be in a different document, then?
Comment 11•19 years ago
|
||
Ah right, now I understand your question. The popupNode or tooltipNode is the target (which may be in a subdocument) of the event that bubbled up to the srcContent thus triggering the popup/tooltip.
Comment 12•19 years ago
|
||
OK. So in other words, right now it's the responsibility of every single caller to nsIPopupBoxObject::ShowPopup to set the popupNode correctly before making the call? Perhaps showPopup should take the node to set as the popupNode as the argument? Alternately, perhaps the code in popupframe should only mess with coordinates if it has both nodes it needs? That way not setting popupNOde would still be ok as long as you're all within a single document. Though that would still break if there are _any_ subdocuments around, since context menus never clear the popupNode they set. Shouldn't the popup coming down, at least, clear the popupNode?
Comment 13•19 years ago
|
||
I don't think I understand your question. The popup/tooltip listeners want to show their popup near the mouse cursor. The information they have is: * The popup to be shown * The mouse event that triggered the popup, which includes * Client X and Y co-ordinates * Target element * Current target (which may be in a different document). But I don't know which element the X and Y coordinates are relative to.
Comment 14•19 years ago
|
||
> I don't think I understand your question. I think that's because we're talking about different things... > The popup/tooltip listeners want to show their popup near the mouse cursor. Sure. That's fine. But I'm talking about popups shown by explicitly calling the showPopup() method on an object implementing the nsIPopupBoxObject interface. that's what this bug is filed on, after all. The way the code is currently written, it seems that properly opening a popup requires 3 DOM nodes: 1) The popup's DOM node (a XUL <popup> element). 2) The "source node" (this is the thing triggering the popup?) 3) The target node (the node that's right-clicked for a context menu popup). The showPopup() method takes _two_ nodes as arguments. It takes a source node as the first argument and the popup's DOM node as the second argument (which seems to just duplicate its mContent, but that's not the issue). Therefore, this method can't possibly properly open a popup on its own. So either all callers need to set the document's popupNode to the target node before calling showPopup(), or showPopup() needs another argument, or something else needs to happen.
Comment 15•19 years ago
|
||
So we need three nodes because the event's coordinates are relative to the target node's document?
Comment 16•19 years ago
|
||
We need three nodes because the popup back end assumes that the passed-in coordinates are relative to the document of the popupNode of the XUL document the popup is in. Nowhere is it documented that any of this has anything to do with events. But yes, for the special case of context menu popups that seems to be the reason.
Comment 17•19 years ago
|
||
(In reply to comment #16) >We need three nodes because the popup back end assumes that the passed-in >coordinates are relative to the document of the popupNode of the XUL document >the popup is in. Is that because that's how our event coordinates behave? > Nowhere is it documented that any of this has anything to do with events. Documented? You're being over-optimistic :-P >But yes, for the special case of context menu popups that seems to be the reason. Don't forget tooltips.
Comment 18•19 years ago
|
||
OK. I see no quick fix here, and further after some thought I've decided that our client* thing for events and current popup setup is fundamentally broken. I've filed bug 291083 on suggested changes that would "fix" this bug (lead to consistent behavior, basically).
Depends on: 291083
Comment 19•19 years ago
|
||
I modified the XUL file and now it works :-)
Comment 20•18 years ago
|
||
I wrote a comment to the related/duplicate bug https://bugzilla.mozilla.org/show_bug.cgi?id=362403#c4 . Will this bug be fixed or should it be made dependent on the bug #279703?
Component: XP Toolkit/Widgets: XUL → XUL
QA Contact: xptoolkit.widgets
Comment 22•9 years ago
|
||
No longer relevant.
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → WONTFIX
You need to log in
before you can comment on or make changes to this bug.
Description
•