588070 - Anonymous tooltips don't work

Status: VERIFIED FIXED

(Core :: XUL, defect)

Description

[[e-mail]]

Prior to bug #383930, tooltips would cache the current target of the mouse event, and set that as the document's tooltipNode while launching the tooltip.

However the new mechanism has the popup manager trying to read the target from the now stale event, and using the tooltip's binding parent instead.

The tooltip's triggerNode is the same as document.popupNode of course.

Example markup:
<content>
  <xul:hbox tooltip="_child">
    <xul:tooltip/>
  </xul:hbox>
</content>

Comment 1

[[e-mail]]

Background information: tabbrowser tab tooltips have stopped working in SeaMonkey. Firefox is not affected because its tabs are no longer anonymous.

Comment 2

[Neil Deakin]

Do you mean that the popup manager should be retrieving the event's originalTarget rather than the event's target?

Comment 3

[[e-mail]]

Well the old behaviour was that the tooltip node was the current target relative to the element with the tooltip attribute. So assuming the original target and the tooltip node are in the same document, we could check the binding parent chain until we got to a node with the same binding parent.

Comment 4

[[e-mail]]

Attachment: Untested fix

Comment 5

[Neil Deakin]

Comment on attachment 466776 [details] [diff] [review]
Untested fix

OK, I think I understand what this is doing now.

Comment 6

[[e-mail]]

Comment on attachment 466776 [details] [diff] [review]
Untested fix

smaug, do you know a better way to do this (work out what the target would have been during the original event dispatch)?

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

So where is the "stale" event stored? If the event is stored somewhere and
popup is triggered using it later, what prevents someone to re-dispatch
it between the original dispatch and launching the popup?

Comment 8

[Neil Deakin]

After some thought, I think it would be better to just add a ShowTooltip method to the popup manager which takes a content node instead of an event, thus avoiding these issues.

Comment 9

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 466776 [details] [diff] [review]
Untested fix

I assume there is a new patch coming to remove the dependency of a dom event.

Comment 10

[[e-mail]]

Attachment: Remove the event

Comment 11

[[e-mail]]

Comment on attachment 468078 [details] [diff] [review]
Remove the event

Seeking approval to fix the regression from the blocking bug 383930.

Comment 12

[Bruno 'Aqualon' Escherl]

Any progress on the approval for 2.0? It would be really nice to have tab tooltip preview back for SeaMonkey.

Comment 13

[Bruno 'Aqualon' Escherl]

Hm, the patch doesn't build anymore due to the changes to FirePopupShowingEvent in bug 558072. Changing the call from

FirePopupShowingEvent(aPopup, nsnull, popupFrame->PresContext(), popupFrame->PopupType(), PR_FALSE, PR_FALSE);

to

FirePopupShowingEvent(aPopup, PR_FALSE, PR_FALSE);

seems to work, I do get the tooltip preview with the patch.

Comment 14

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 468078 [details] [diff] [review]
Remove the event

a=beltzner

Comment 15

[[e-mail]]

Pushed changeset e6086627037c to mozilla-central.

Comment 17

[Bruno 'Aqualon' Escherl]

Verified fixed with SeaMonkey 2.1b1pre, built from http://hg.mozilla.org/mozilla-central/rev/2754923dff6e