Closed Bug 588070 Opened 10 years ago Closed 10 years ago

Anonymous tooltips don't work

Categories

(Core :: XUL, defect)

defect
Not set

Tracking

()

VERIFIED FIXED

People

(Reporter: neil, Assigned: neil)

References

Details

(Keywords: regression)

Attachments

(2 files)

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>
Background information: tabbrowser tab tooltips have stopped working in SeaMonkey. Firefox is not affected because its tabs are no longer anonymous.
Blocks: 588018
Do you mean that the popup manager should be retrieving the event's originalTarget rather than the event's target?
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.
Attached patch Untested fixSplinter Review
Attachment #466776 - Flags: feedback?(enndeakin)
Comment on attachment 466776 [details] [diff] [review]
Untested fix

OK, I think I understand what this is doing now.
Attachment #466776 - Flags: feedback?(enndeakin) → feedback+
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)?
Attachment #466776 - Flags: review?(Olli.Pettay)
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?
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 on attachment 466776 [details] [diff] [review]
Untested fix

I assume there is a new patch coming to remove the dependency of a dom event.
Attachment #466776 - Flags: review?(Olli.Pettay)
Attached patch Remove the eventSplinter Review
Assignee: nobody → neil
Status: NEW → ASSIGNED
Attachment #468078 - Flags: review?(enndeakin)
Attachment #468078 - Flags: review?(enndeakin) → review+
Comment on attachment 468078 [details] [diff] [review]
Remove the event

Seeking approval to fix the regression from the blocking bug 383930.
Attachment #468078 - Flags: approval2.0?
Any progress on the approval for 2.0? It would be really nice to have tab tooltip preview back for SeaMonkey.
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 on attachment 468078 [details] [diff] [review]
Remove the event

a=beltzner
Attachment #468078 - Flags: approval2.0? → approval2.0+
Pushed changeset e6086627037c to mozilla-central.
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Duplicate of this bug: 588018
Verified fixed with SeaMonkey 2.1b1pre, built from http://hg.mozilla.org/mozilla-central/rev/2754923dff6e
Status: RESOLVED → VERIFIED
Depends on: 595570
Depends on: 599409
Depends on: 644952
You need to log in before you can comment on or make changes to this bug.