Closed
Bug 82597
Opened 23 years ago
Closed 23 years ago
event.client[X|Y] not filled when creating tooltip popup
Categories
(Core :: XUL, defect)
Tracking
()
VERIFIED
WONTFIX
People
(Reporter: janv, Assigned: janv)
Details
Attachments
(8 files)
(deleted),
text/plain
|
Details | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review |
Testcase is coming...
Assignee | ||
Comment 1•23 years ago
|
||
Assignee | ||
Comment 2•23 years ago
|
||
Assignee | ||
Comment 3•23 years ago
|
||
There is still a small problem to fix.
Tooltip should disappear on mouse move.
Mike, can you take a look ?
No longer blocks: 73865
Status: NEW → ASSIGNED
Comment 5•23 years ago
|
||
oops, sorry, thought this was on my list.
Target Milestone: mozilla1.0 → ---
Comment 6•23 years ago
|
||
tooltips go away when the mouse leaves the frame associated with the content node
that the mouse was over when it appeared (event.target). they don't go away on
any ol' mouse move. since outliner is one frame, the tip won't go away until you
leave the outliner.
Assignee | ||
Comment 7•23 years ago
|
||
Assignee | ||
Comment 8•23 years ago
|
||
Assignee | ||
Comment 9•23 years ago
|
||
I've attached better fix.
Mike, note this:
- self->LaunchPopup ( self->mMouseClientX, self->mMouseClientY+21);
+ self->LaunchPopup ( self->mMouseClientX, self->mMouseClientY+21,
+ self->mMouseClientX, self->mMouseClientY );
Comment 10•23 years ago
|
||
Mike, we need this for the folder_outliner branch which now is nearly ready for
landing. Can you review the attached fix?
Assignee | ||
Comment 11•23 years ago
|
||
Assignee | ||
Comment 12•23 years ago
|
||
Comment 13•23 years ago
|
||
as we discussed before, the +21 is a hack and i can guarantee won't work in all
cases since it's probably the offset of this particular frame in its view. You
really need to find a programmatic way to come up with this 21 value, probably
by walking the parent chain (not sure view or widget) and adding the offsets.
Assignee | ||
Comment 14•23 years ago
|
||
Mike, did you take a look at last patches ?
It works for me with those patches.
Comment 15•23 years ago
|
||
yes, i looked at the patches. i don't like this:
+ else if ( popupType == eXULPopupType_tooltip ) {
type.AssignWithConversion("tooltip");
+ yDelta += 21;
+ }
you need to come up with a programmatic way of computing the 21.
Assignee | ||
Comment 16•23 years ago
|
||
But it was hardcoded this way also before:
- self->LaunchPopup ( self->mMouseClientX, self->mMouseClientY+21);
+ self->LaunchPopup ( self->mMouseClientX, self->mMouseClientY )
I'm sorry but I'm not sure what do you mean by "programmatic way" ?
Comment 17•23 years ago
|
||
oh dear lord, you're right. sorry about that. we really need to explain the
origin of that constant, but that can be left for me to do.
I'll look at the patch again later today, and this time i'll promise to turn my
brain on first ;)
Comment 18•23 years ago
|
||
r=pink on the code. sorry for the delay and being so braindead.
I would prefer a name other than |xDelta| and |yDelta| for the params to
createPopup. Any ideas? How about |xAdjust| and |yAdjust| since that's really
what these params are doing: adjusting the popup position for various circumstances.
Assignee | ||
Comment 19•23 years ago
|
||
Assignee | ||
Comment 20•23 years ago
|
||
Assignee | ||
Comment 21•23 years ago
|
||
I've renamed delta to adjust according to Mike's suggestion.
Comment 22•23 years ago
|
||
I do not agree with this patch. (Sorry!) I don't believe any additional
arguments should be added to createPopup. The "create" and "destroy" events are
not intended to be mouse events either, since they can be invoked via the
keyboard or programmatic means.
In the interest of understanding this, could you explain
(a) Why the create/destroy events would ever need mouse coords?
(b) What is the purpose of the adjust args that have been added?
Assignee | ||
Comment 23•23 years ago
|
||
>(a) Why the create/destroy events would ever need mouse coords?
We have tooltip popup in folderPane.xul:
<popup id="folderTooltip" class="tooltip" oncreate="return
FillInFolderTooltip(event);">
<text id="folderTooltipText"/>
<popup>
When this tooltip is created we need to know mouse coordinates to find out
corresponding row index of outliner:
function FillInFolderTooltip(event) {
...
getCellAt(event.clientX, event.clientYdocument.tooltipNode, row, col, elt)
var folderResource = GetFolderResource(row.value);
var msgFolder =
folderResource.QueryInterface(Components.interfaces.nsIMsgFolder);
var unreadCount = msgFolder.getNumUnread(false);
var totalCount = msgFolder.getTotalMessages(false);
var folderTooltip = GetFolderAttribute(folderResource, 'Name');
folderTooltip += " (" + unreadCount + "/" + totalCount +")";
textNode.setAttribute('value', folderTooltip);
return true;
}
Before it was easier because we used "document.tooltipNode" to access
corresponding <treecell>. But now there is only one frame <outlinerbody>
>(b) What is the purpose of the adjust args that have been added?
One problem was to get mouse coordinates, another problem was that those
coordinates was slightly "adjusted". Fox example y position is adjusted by 21
pixels to display *tooltip* popups and context popups are adjusted by 2 pixels
in both directions.
This have been done *before* passing to method createPopup (in nsXULPopupListener)
Therefore we had to add new parameters xAdjust and yAdjust to fill mouse event
with correct values
I don't know if there is better solution
Assignee | ||
Comment 24•23 years ago
|
||
sorry, I messed my comment a little bit
-getCellAt(event.clientX, event.clientYdocument.tooltipNode, row, col, elt)
+getCellAt(event.clientX, event.clientY, row, col, elt)
Assignee | ||
Comment 25•23 years ago
|
||
It seems that we have a better solution.
Marking as WONTFIX.
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 26•23 years ago
|
||
marked as fixed by mistake
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Comment 27•23 years ago
|
||
wontfix
Status: REOPENED → RESOLVED
Closed: 23 years ago → 23 years ago
Resolution: --- → WONTFIX
Comment 28•23 years ago
|
||
Which way to the better solution? (just curious, bug number?)
Status: RESOLVED → VERIFIED
Assignee | ||
Comment 29•23 years ago
|
||
See last comments in 73865.
You need to log in
before you can comment on or make changes to this bug.
Description
•