Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#7361 closed defect (fixed)

Tooltip: In 2D charts with event support, tooltips sometimes remain unduly hoisted

Reported by: enzo Owned by: bill
Priority: high Milestone: 1.2
Component: Dijit Version: 1.1.1
Keywords: Cc:
Blocked By: Blocking:

Description

Here is an example: open page http://archive.dojotoolkit.org/nightly/dojotoolkit/dojox/charting/tests/test_event2d.html , go down to Chart 10, place the mouse pointer over the green slice, then quickly move it down transiting through the red slice: the “Red is 50%” tooltip remains hoisted. This also happens with bar charts.

I have not attempted a debugging, but it appears that the problem arises if the "mouseout" for the second slice occurs while the tooltip of the first slice is stil hoisted. Probably the "mouseout" events from both slices get addressed to the first tooltip (redundantly), and the second tooltip never receives any "mouseout".

Change History (12)

comment:1 Changed 11 years ago by Eugene Lazutkin

Milestone: tbdfuture

Moving all open ticketd to the future.

comment:2 in reply to:  1 Changed 11 years ago by enzo

Replying to elazutkin:

Moving all open ticketd to the future.

Actually, this problem applies to any kind of tooltip, so probably its resolution should be delegated to the relative development team.

comment:3 Changed 11 years ago by Eugene Lazutkin

Owner: changed from Eugene Lazutkin to bill

There is no way to account for lost mouse events. One way to "solve" it is to make it go out on a timer, if mouse outside of it.

Let me redirect it to Bill.

comment:4 Changed 11 years ago by Eugene Lazutkin

Component: ChartingDijit

comment:5 Changed 11 years ago by bill

Milestone: future1.3
Summary: In 2D charts with event support, tooltips sometimes remain unduly hoistedTooltip: In 2D charts with event support, tooltips sometimes remain unduly hoisted

Hmm, OK, yah I'll take a look at it. Doesn't sound like a dropped event to me but rather what the original description says... but of course won't know for sure until looking.

comment:6 in reply to:  5 Changed 11 years ago by enzo

Replying to bill:

Hmm, OK, yah I'll take a look at it. Doesn't sound like a dropped event to me but rather what the original description says... but of course won't know for sure until looking.

OK, SOLVED! The problem goes beyond Charting, and affects all applications of Tooltips, especially in high-density scenarios (e.g., tooltips on DataGrid? cells).

It all depends on a wrong sequence of instructions in dijit/Tooltip.js. This:

  hide: function(aroundNode){
    // summary: hide the tooltip
    if(!this.aroundNode || this.aroundNode !== aroundNode){
      return;
    }
    if(this._onDeck){
      // this hide request is for a show() that hasn't even started yet;
      // just cancel the pending show()
      this._onDeck=null;
      return;
    }

should really be:

  hide: function(aroundNode){
    // summary: hide the tooltip
    if(this._onDeck){
      // this hide request is for a show() that hasn't even started yet;
      // just cancel the pending show()
      this._onDeck=null;
      return;
    }
    if(!this.aroundNode || this.aroundNode !== aroundNode){
      return;
    }

...or else the cancellation of the pending show request will sometimes be missed, and nobody will hide the tooltip after its scheduled show.

After this change, tooltips behave perfectly.

comment:7 Changed 11 years ago by bill

OK, great catch!

I suppose technically the condition should be something like:

if(this._onDeck && this._onDeck[1] == aroundNode)

so that random hide() calls (for neither the first slice or second slice in your original bug description) are still ignored.

Actually a better function def might be:

hide: function(aroundNode){
	// summary: hide the tooltip
	if(this._onDeck && this._onDeck[1] == aroundNode){
		// this hide request is for a show() that hasn't even started yet;
		// just cancel the pending show()
		this._onDeck=null;
	}else if(this.aroundNode === aroundNode){
		// this hide request is for the currently displayed tooltip
		this.fadeIn.stop();
		this.isShowingNow = false;
		this.aroundNode = null;
		this.fadeOut.play();
	}else{
		// just ignore the call, it's for a tooltip that has already been erased
	}
},

I'm trying that now but the problem is that from my console.log() statements, apparently FF3/mac *is* dropping events. Moving the mouse quickly over the input boxes in test_Tooltip I see this output (after adding console.logs):

id1_tooltip : onHover  undefined undefined
mouseover clientX=112, clientY=165 <input id="id1" value="#1"> mouseover
id3_tooltip : onHover undefined undefined
mouseout clientX=96, clientY=176 <input id="id4" value="#4"> mouseout
id3_tooltip : onUnHover undefined 12
mouseout clientX=81, clientY=197 <input id="id5" value="#5"> mouseout
id4_tooltip : onUnHover undefined undefined
mouseout clientX=80, clientY=218 <input id="id6" value="#6"> mouseout
id5_tooltip : onUnHover undefined undefined
id1_tooltip opened against target <input id="id1" value="#1">

comment:8 Changed 11 years ago by bill

(In [15246]) Add id's to tooltips for easier debugging. Refs #7361.

comment:9 in reply to:  7 ; Changed 11 years ago by enzo

Replying to bill:

Actually a better function def might be: [...]

But might any problem arise by just using my proposed code change? It seems to work flawlessly for me, with FF3, MSIE6 and MSIE7 on WinXP.

Enzo

comment:10 in reply to:  9 Changed 11 years ago by bill

Replying to enzo:

But might any problem arise by just using my proposed code change? It seems to work flawlessly for me, with FF3, MSIE6 and MSIE7 on WinXP.

The only problem is when an unrelated widget calls hide(myAroundNode) at just the wrong time. Remember that there's a single MasterTooltip for the page that's shared between all the Tooltip widgets and ValidationTextBox and perhaps other clients.... so some obscure case like this:

  1. FilteringSelect widget is displaying tooltip showing invalid input error
  2. user tabs away from FilteringSelect widget causing it to send XHR to the server to test whether new input is valid
  3. user mouses over nodeA, and FilteringSelect tooltip disappears because nodeA's tooltip appears. FilteringSelect, however, thinks that it's tooltip is still being shown.
  4. user moves mouse over nodeB, and nodeA's tooltip starts to fade out (nodeB's tooltip is put onDeck)
  5. FilteringSelect gets answer from the server... new input is OK so it decides to erase it's tooltip (it doesn't realize that it's already been erased). FilteringSelect calls hideTooltip(myDomNode)
  6. the onDeck tooltip for nodeB is canceled, and it never appears

I'm not sure if that would even happen, but anyway back in the old days there was a reason that the hide() method took a aroundNode parameter and it could be dangerous to ignore it.

comment:11 Changed 11 years ago by bill

Resolution: fixed
Status: newclosed

(In [15248]) Fixes #7361: race condition leading to tooltip remaining shown when it shouldn't have appeared/should have been erased:

  1. tooltipA is being shown
  2. hide() is called for tooltipA; tooltipA starts to fade out
  3. show() is called for tooltipB; tooltipB is put onDeck
  4. hide() is called for tooltipB (while it's still onDeck)
  5. tooltipB is shown

I still see dropped mouseover/mouseout events on FF (although not on other browsers) that could potentially cause similar problems... seems impossible to avoid that problem completely although I think I could minimize it by refactoring Tooltip to have a single mouseout/mouseover handler on <body> and track the active node (similar to focus manager). Will leave that for another release if it proves to be necessary.

!strict

comment:12 Changed 11 years ago by bill

Milestone: 1.31.2
Note: See TracTickets for help on using tickets.