Skip to content
This repository has been archived by the owner on May 29, 2019. It is now read-only.

Fixed: TypeError: Cannot read property 'offsetHeight' of null #6226

Closed
wants to merge 2 commits into from

Conversation

Johnnyrook777
Copy link
Contributor

@Johnnyrook777 Johnnyrook777 commented Sep 7, 2016

If the tooltip is hidden before the timeout resolves, an exception is created. #6221

Fix is to cancel the timeout when the tooltip is hidden

If the tooltip is hidden before the timeout resolves, an exception is created.

Fix is the cancel the timeout when the tooltip is hidden
@tkenakka
Copy link

tkenakka commented Sep 14, 2016

Hi, great to see there's a fix candidate. Any idea when this will be merged and distributed via a new bower package?
We're reproducing this problem randomly, causing problems for us.

@rarkins
Copy link

rarkins commented Sep 14, 2016

Same here, but we use npm

@Rouche
Copy link
Contributor

Rouche commented Sep 21, 2016

@Johnnyrook777 : Hello, did you try your fix against this Plunk : http://embed.plnkr.co/P6bLrajSHzkTdftcMHTw/

Just to be sure there is no race conditions deeper with $destroy.

@@ -315,6 +317,11 @@ angular.module('ui.bootstrap.tooltip', ['ui.bootstrap.position', 'ui.bootstrap.s
$timeout.cancel(transitionTimeout);
transitionTimeout = null;
}

if (positionTimeout) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't we want to keep it if the hide is canceled?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was something I added thinking there was may be a left over timeout and potential memory leak
However, it isn't actually part of the issue, and was more of a memory tweak.

Although it didn't cause issues in my scenario, it probably requires more testing

Copy link
Contributor Author

@Johnnyrook777 Johnnyrook777 Oct 5, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I took another look at this. The error happens when the tooltip is positioned or adjusted, but the DOM has been destroyed/unloaded somehow (For me occurring during a route change)

I suspect this timeout should be cancelled as well as the DOM is definitely gone, so this position code has nothing to run on anyway

Either way, this error is hugely timing dependant

@wesleycho

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually. Thought about this more on the way home. I suspect this condition could never occur, and there are no reports of it happening. Don't change what isn't broke

I'll update this tomorrow and remove this

@wesleycho
Copy link
Contributor

Made the change locally, merging shortly.

@wesleycho wesleycho added this to the 2.2.0 milestone Oct 10, 2016
@wesleycho wesleycho closed this in 7855976 Oct 10, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants