Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Crash When Showing but No Active Hover: Cannot read properties of null (reading 'data') #184

Open
speedplane opened this issue Jan 15, 2023 · 7 comments
Assignees
Labels
Bug Software defects or other problems that should be fixed.

Comments

@speedplane
Copy link

speedplane commented Jan 15, 2023

I came across a crash while hovering over a number of items that have powertips. This is some sort of open/close race condition, because it doesn't happen often, although I can reproduce it somewhat easily.

The error message is: Cannot read properties of null (reading 'data')

The image below illustrates the moment of the crash, the tip is open, but there is no active hover. Can it simply be fixed by changing:

				if (!session.isClosing) {
					hideTip(session.activeHover);
				}

to

				if (!session.isClosing && session.activeHover) {
					hideTip(session.activeHover);
				}

Using version 1.3.2

image

image

Callstack

hideTip (jquery.powertip.js:1021)
showTip (jquery.powertip.js:912)
queueTipInit (jquery.powertip.js:886)
dequeue (jquery.js)
r.complete (jquery.js)
l (jquery.js)
add (jquery.js)
Kn (jquery.js)
o (jquery.js)
dequeue (jquery.js)
o (jquery.js)
queueTipInit (jquery.powertip.js:887)
dequeue (jquery.js)
o (jquery.js)
queueTipInit (jquery.powertip.js:887)
dequeue (jquery.js)
r.complete (jquery.js)
l (jquery.js)
fireWith (jquery.js)
a (jquery.js)
v.fx.tick (jquery.js)
setInterval (async)
v.fx.timer (jquery.js)
Kn (jquery.js)
o (jquery.js)
dequeue (jquery.js)
o (jquery.js)
queueTipInit (jquery.powertip.js:887)
dequeue (jquery.js)
(anonymous) (jquery.js)
each (jquery.js)
each (jquery.js)
queue (jquery.js)
beginShowTip (jquery.powertip.js:885)
openTooltip (jquery.powertip.js:505)
apiShowTip (jquery.powertip.js:251)
show_docket_tip (user_code)

@stevenbenner
Copy link
Owner

Thanks for reporting the issue. Your suggested change seems like a good sanity check, but I'm hesitant to make such a change without first understanding the root cause.

A situation where PowerTip believes that a tooltip is open but doesn't know of any element that triggered it could cause other problems. Adding a sanity check like this could mask a more fundamental bug and make it harder to find the root cause in the future if someone runs into a different side-effect of the same root problem. So my goal would be to prevent it from getting in this unexpected state in the first place.

A couple requests:

  • Can you provide steps for me to reproduce the issue (including setup code)?
  • And/or provide a simple test page for me to reproduce the problem?

I'm interested in seeing the code used so I can better understand the paths that are being followed and how state is being changed along the way.

It would also help me debug if you could answer these questions:

  1. How many elements with tooltips are there on the page?
  2. Does there seem to be a minimum number of elements tooltips to make this happen?
  3. You said "hovering over a number of items that have powertips", do you mean that there are multiple elements that are nested?
    (e.g. <div title="tip1">some text <span title="tip2">more text</span></div>)
  4. Are you using the mouse cursor or keyboard cursor when this happens?
  5. Does this happen when you move the cursor slowly from one tooltip to another or does it need to be fast?
  6. Do you ever manually call the PowerTip hide method on the page?
  7. Do you ever call the PowerTip destroy method on the page?
  8. Do you ever remove DOM elements that have or previously had tooltips assigned to them?
  9. Do you use any custom PowerTip events?
  10. Do you use any custom PowerTip integration (i.e. manual: true)?
  11. Does this happen in both Chrome and Firefox?

@speedplane
Copy link
Author

speedplane commented Feb 23, 2023

Hi, Steven, it took some time to get back to this.

Video summary

I created a 2:00 video that should hopefully answer these questions and more: https://www.youtube.com/watch?v=gTexPznr8KY

Link to the page in question: https://www.docketalarm.com/cases/Michigan_Eastern_District_Court/1--20-cv-13025/B%26P_Littleford_LLC_v._Prescott_Machine_LLC_et_al/

Answers to your questions.

  1. How many elements with tooltips are there on the page? - Many
  2. Does there seem to be a minimum number of elements tooltips to make this happen? Not sure.
  3. You said "hovering over a number of items that have powertips", do you mean that there are multiple elements that are nested? No ... no nested powertips that I'm aware of.
  4. Are you using the mouse cursor or keyboard cursor when this happens? Mouse cursor.
  5. Does this happen when you move the cursor slowly from one tooltip to another or does it need to be fast? Generally happens quickly.
  6. Do you ever manually call the PowerTip hide method on the page? Yes... is that bad?
  7. Do you ever call the PowerTip destroy method on the page? Yes, but only when powertip crashes.
  8. Do you ever remove DOM elements that have or previously had tooltips assigned to them? I don't think this is happening.
  9. Do you use any custom PowerTip events? No.
  10. Do you use any custom PowerTip integration (i.e. manual: true)? No.
  11. Does this happen in both Chrome and Firefox? So far, only Chrome. I've tried and been unable to reproduce in Firefox.

Follow-Up

I realize that this issue may be a duplicate, of the much older #169

@speedplane
Copy link
Author

speedplane commented Feb 27, 2023

Hi Steven,

I have an easier way to reproduce:

  1. View this page
  2. Hover over link in the center of the page
  3. See error

Unfortunately, the production page is minified and does not have debug info, so the callstack is unreadable, but it is the same as the callstack shown in the video with the debug info.

image

@speedplane
Copy link
Author

Here is the patch I will be making, with this, the bug goes away (and you won't be able to reproduce at the link above):
image

@stevenbenner
Copy link
Owner

Thank you for the additional information. That was very helpful. Especially the example link and video. I really appreciate you putting in the effort to put all of that together for me. The critical piece of information was that tooltips are being hidden then immediately shown again in the same function (your set_docket_tip() function). I would not have considered that possibility without the link and video.

I believe that I have found the issue.

Test case

Please try out my reduced test case to see if this matches the issue that you had been seeing:

Issue

It appears that the PowerTip state can be put in an unexpected state if the API hide() method is invoked with immediate set to true at exactly the right (wrong) time during the tooltip life cycle.

I am able to reproduce this issue by rapidly showing and hiding a tooltip multiple times in quick succession. The linked test case demonstrates the minimum action needed to trigger this issue.

Repro steps

  1. (start with open tooltip)
  2. Call API hide() method with immediate set to true
  3. Call API show() method
  4. Call API hide() method with immediate set to true
  5. Call API show() method

Repro rate is 100% in this case. For both Firefox and Chromium.

This cannot be reproduced if immediate is not set to true in at least one case. If only one hide() call has immediate set to true then it is more difficult to reproduce, but can be done by rapidly clicking the button in the reduced test case.

Diagnosis

This is basically a race condition.

I was able to determine that it is possible for the fadeOutCallback() to be queued and executed immediately after TooltipController.showTip() completes. This can happen if the API hide() method is called in the very narrow window of time during the tooltip being queued for open, but before showTip() is triggered.

Flow

Step Function Description activeHover
1. n/a (start with open tooltip) set
2. hideTip() API asks to close open tip (dev plans to update content and reopen the tip). Queues up the fadeOut() with state reset. set
3. beginShowTip() API immediate asks to open new tip, adds showTip() to the queue (for content update from step 2). Queues up showTip(). set
4. hideTip() Another call to close a tooltip is triggered (maybe unrelated API or user mouse moves away). Queues up another fadeOut() with state reset. set
5. beginShowTip() Second request to open tooltip occurs (maybe unrelated API or user mouse moves to another element). Queues up another showTip(). set
6. fadeOutCallback() Queue moves forward from step 2. The fadeOut() is completed and state is reset. null
7. showTip() Queue moves forward one tick from step 3. PowerTip begins opening a tooltip and sets session.activeHover and session.isTipOpen. set
8. fadeOutCallback() Queue moves forward from step 7. The fadeOut() requested in step 4 is completed and state is reset. null
9. showTip() Queue moves forward one tick from step 8, and runs the request from step 5. The showTip() method finds that a tooltip is open/opening (session.isTipOpen is true) from step 7 and tries to close it, calling hideTip(). null
10. hideTip() Error! The session.activeHover was cleared in step 8. PowerTip has been asked to close a tooltip when there are none open. null

Remedy

I think that the bug here is that opening tooltips is part of jQuery's .queue() system, but hiding tooltips is not when immediate set to true on the API hide() method. This allows operations to become out of order if hide() is called while there are functions waiting in the queue.

A simple patch can make the hide() calls be handled by the queue. Though, this will need some refactoring of the destroy() API method to make it fully compatible.

diff --git a/src/tooltipcontroller.js b/src/tooltipcontroller.js
index 136c696..3ca1eec 100644
--- a/src/tooltipcontroller.js
+++ b/src/tooltipcontroller.js
@@ -171,12 +173,26 @@ function TooltipController(options) {
         });
     }
 
+    /**
+     * Queues up the hideTip function.
+     * @private
+     * @param {jQuery} element The element with the tooltip to hide.
+     */
+    function beginHideTip(element) {
+        // hide tooltip, asap
+        tipElement.queue(function queueTipHide(next) {
+            hideTip(element);
+            next();
+        });
+    }
+
     /**
      * Hides the tooltip.
      * @private
@@ -415,6 +432,6 @@ function TooltipController(options) {
 
     // expose methods
     this.showTip = beginShowTip;
-    this.hideTip = hideTip;
+    this.hideTip = beginHideTip;
     this.resetPosition = positionTipOnElement;
 }

I suspect that this patch would fix the issue that you've been seeing. Could you give it a try?

@stevenbenner stevenbenner added the Bug Software defects or other problems that should be fixed. label Mar 20, 2023
@stevenbenner stevenbenner self-assigned this Mar 20, 2023
@speedplane
Copy link
Author

speedplane commented May 11, 2023

Hi @stevenbenner,

Apologies on the long iteration times. I applied your patch, removed my null checks, and unfortunately, I still see the exception.

Screenshot of New Crash

Notes on Screenshot

  • Again, this occurred when multiple powertips were opening/closing, somewhat intermittent
  • Exception corresponds to the second null check I added in the screenshot above
  • Note line 1273: It has your new patch
  • I hovered over the session variable so you can see it's contents, but happy to provide additional info.

@speedplane
Copy link
Author

speedplane commented May 11, 2023

Another video demo

Powertip Library - Demo of Crash - 2nd Iteration - 2023-05-11

Notes:

  • In the video, the procedure is to:
    • hover over a button that has a powerTip
    • press it
    • the resulting action of pressing the button causes a 2nd powertip to be triggered
    • There are other tips on the page, but I think these are the main 2.
  • I applied your patch to the code in this video
  • For reference, below is a screenshot of the changes in my checkout

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Software defects or other problems that should be fixed.
Projects
None yet
Development

No branches or pull requests

2 participants