From 4081c013c142b52fac914b91fc1af9a2cce9a98d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jacob=20M=C3=BCller?= Date: Tue, 19 Nov 2019 08:45:13 +0100 Subject: [PATCH 1/7] fix(v-b-tooltip): memory leaks --- src/components/tooltip/helpers/bv-tooltip.js | 42 ++++++++++++-------- 1 file changed, 25 insertions(+), 17 deletions(-) diff --git a/src/components/tooltip/helpers/bv-tooltip.js b/src/components/tooltip/helpers/bv-tooltip.js index 63f4f758113..b8cff8664f6 100644 --- a/src/components/tooltip/helpers/bv-tooltip.js +++ b/src/components/tooltip/helpers/bv-tooltip.js @@ -237,9 +237,9 @@ export const BVTooltip = /*#__PURE__*/ Vue.extend({ this.unListen() this.setWhileOpenListeners(false) - // Clear any timeouts/Timers - clearTimeout(this.$_hoverTimeout) - this.$_hoverTimeout = null + // Clear any timeouts/intervals + this.clearHoverTimeout() + this.clearVisibilityInterval() this.destroyTemplate() this.restoreTitle() @@ -271,6 +271,18 @@ export const BVTooltip = /*#__PURE__*/ Vue.extend({ this.fixTitle() } }, + clearHoverTimeout() { + if (this.$_hoverTimeout) { + clearTimeout(this.$_hoverTimeout) + this.$_hoverTimeout = null + } + }, + clearVisibilityInterval() { + if (this.$_visibleInterval) { + clearInterval(this.$_visibleInterval) + this.$_visibleInterval = null + } + }, createTemplateAndShow() { // Creates the template instance and show it // this.destroyTemplate() @@ -324,11 +336,9 @@ export const BVTooltip = /*#__PURE__*/ Vue.extend({ // The `hook:destroyed` will also be called (safety measure) this.$_tip && this.$_tip.hide() }, + // Destroy the template instance and reset state destroyTemplate() { - // Destroy the template instance and reset state - this.setWhileOpenListeners(false) - clearTimeout(this.$_hoverTimeout) - this.$_hoverTimout = null + this.clearHoverTimeout() this.$_hoverState = '' this.clearActiveTriggers() this.localPlacementTarget = null @@ -450,8 +460,7 @@ export const BVTooltip = /*#__PURE__*/ Vue.extend({ // This is also done in the template `hide` evt handler this.setWhileOpenListeners(false) // Clear any hover enter/leave event - clearTimeout(this.hoverTimeout) - this.$_hoverTimeout = null + this.clearHoverTimeout() this.$_hoverState = '' this.clearActiveTriggers() // Disable the fade animation on the template @@ -712,14 +721,13 @@ export const BVTooltip = /*#__PURE__*/ Vue.extend({ // On-touch start listeners this.setOnTouchStartListener(on) }, + // Handler for periodic visibility check visibleCheck(on) { - // Handler for periodic visibility check - clearInterval(this.$_visibleInterval) - this.$_visibleInterval = null + this.clearVisibilityInterval() const target = this.getTarget() const tip = this.getTemplateElement() if (on) { - this.visibleInterval = setInterval(() => { + this.$_visibleInterval = setInterval(() => { if (tip && this.localShow && (!target.parentNode || !isVisible(target))) { // Target element is no longer visible or not in DOM, so force-hide the tooltip this.forceHide() @@ -884,14 +892,14 @@ export const BVTooltip = /*#__PURE__*/ Vue.extend({ this.$_hoverState = 'in' return } - clearTimeout(this.hoverTimeout) + this.clearHoverTimeout() this.$_hoverState = 'in' if (!this.computedDelay.show) { this.show() } else { // Hide any title attribute while enter delay is active this.fixTitle() - this.hoverTimeout = setTimeout(() => { + this.$_hoverTimeout = setTimeout(() => { /* istanbul ignore else */ if (this.$_hoverState === 'in') { this.show() @@ -917,12 +925,12 @@ export const BVTooltip = /*#__PURE__*/ Vue.extend({ if (this.isWithActiveTrigger) { return } - clearTimeout(this.hoverTimeout) + this.clearHoverTimeout() this.$_hoverState = 'out' if (!this.computedDelay.hide) { this.hide() } else { - this.$hoverTimeout = setTimeout(() => { + this.$_hoverTimeout = setTimeout(() => { if (this.$_hoverState === 'out') { this.hide() } From bbc9dcb0fd4ecd80c22e7fca4536b5a5ff9a2c8e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jacob=20M=C3=BCller?= Date: Tue, 19 Nov 2019 08:53:22 +0100 Subject: [PATCH 2/7] Update bv-tooltip.js --- src/components/tooltip/helpers/bv-tooltip.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/components/tooltip/helpers/bv-tooltip.js b/src/components/tooltip/helpers/bv-tooltip.js index b8cff8664f6..b0cc454e5e1 100644 --- a/src/components/tooltip/helpers/bv-tooltip.js +++ b/src/components/tooltip/helpers/bv-tooltip.js @@ -6,6 +6,7 @@ import Vue from '../../../utils/vue' import getScopId from '../../../utils/get-scope-id' import looseEqual from '../../../utils/loose-equal' +import noop from '../../../utils/noop' import { arrayIncludes, concat, from as arrayFrom } from '../../../utils/array' import { isElement, @@ -203,7 +204,7 @@ export const BVTooltip = /*#__PURE__*/ Vue.extend({ this.$_hoverState = '' this.$_visibleInterval = null this.$_enabled = !this.disabled - this.$_noop = () => {} + this.$_noop = noop // Destroy ourselves when the parent is destroyed if (this.$parent) { From 190ac073085a3ac6ade6b1804de0bff24997d857 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jacob=20M=C3=BCller?= Date: Tue, 19 Nov 2019 09:04:34 +0100 Subject: [PATCH 3/7] Update bv-tooltip.js --- src/components/tooltip/helpers/bv-tooltip.js | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/components/tooltip/helpers/bv-tooltip.js b/src/components/tooltip/helpers/bv-tooltip.js index b0cc454e5e1..b7ce9767a3a 100644 --- a/src/components/tooltip/helpers/bv-tooltip.js +++ b/src/components/tooltip/helpers/bv-tooltip.js @@ -336,6 +336,10 @@ export const BVTooltip = /*#__PURE__*/ Vue.extend({ // then emit the `hidden` event once it is fully hidden // The `hook:destroyed` will also be called (safety measure) this.$_tip && this.$_tip.hide() + // Clear out any stragging active triggers + this.clearActiveTriggers() + // Reset the hover state + this.$_hoverState = '' }, // Destroy the template instance and reset state destroyTemplate() { @@ -444,11 +448,6 @@ export const BVTooltip = /*#__PURE__*/ Vue.extend({ // Tell the template to hide this.hideTemplate() - // TODO: The following could be added to `hideTemplate()` - // Clear out any stragging active triggers - this.clearActiveTriggers() - // Reset the hover state - this.$_hoverState = '' }, forceHide() { // Forcefully hides/destroys the template, regardless of any active triggers From 01c91bd4873f8b7c623df71302b59631523abf3e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jacob=20M=C3=BCller?= Date: Tue, 19 Nov 2019 09:18:20 +0100 Subject: [PATCH 4/7] Update bv-tooltip.js --- src/components/tooltip/helpers/bv-tooltip.js | 98 +++++++------------- 1 file changed, 34 insertions(+), 64 deletions(-) diff --git a/src/components/tooltip/helpers/bv-tooltip.js b/src/components/tooltip/helpers/bv-tooltip.js index b7ce9767a3a..12ced0fe3f6 100644 --- a/src/components/tooltip/helpers/bv-tooltip.js +++ b/src/components/tooltip/helpers/bv-tooltip.js @@ -35,8 +35,8 @@ import { import { keys } from '../../../utils/object' import { warn } from '../../../utils/warn' import { BvEvent } from '../../../utils/bv-event.class' - import { BVTooltipTemplate } from './bv-tooltip-template' +import { template } from 'handlebars' const NAME = 'BVTooltip' @@ -237,18 +237,14 @@ export const BVTooltip = /*#__PURE__*/ Vue.extend({ // Remove all handler/listeners this.unListen() this.setWhileOpenListeners(false) - // Clear any timeouts/intervals this.clearHoverTimeout() this.clearVisibilityInterval() - + // Destroy the template this.destroyTemplate() - this.restoreTitle() }, methods: { - // - // Methods for creating and destroying the template - // + // --- Methods for creating and destroying the template --- getTemplate() { // Overridden by BVPopover return BVTooltipTemplate @@ -272,21 +268,8 @@ export const BVTooltip = /*#__PURE__*/ Vue.extend({ this.fixTitle() } }, - clearHoverTimeout() { - if (this.$_hoverTimeout) { - clearTimeout(this.$_hoverTimeout) - this.$_hoverTimeout = null - } - }, - clearVisibilityInterval() { - if (this.$_visibleInterval) { - clearInterval(this.$_visibleInterval) - this.$_visibleInterval = null - } - }, createTemplateAndShow() { // Creates the template instance and show it - // this.destroyTemplate() const container = this.getContainer() const Template = this.getTemplate() const $tip = (this.$_tip = new Template({ @@ -351,6 +334,8 @@ export const BVTooltip = /*#__PURE__*/ Vue.extend({ this.$_tip && this.$_tip.$destroy() } catch {} this.$_tip = null + this.removeAriaDescribedby() + this.restoreTitle() this.localShow = false }, getTemplateElement() { @@ -370,13 +355,10 @@ export const BVTooltip = /*#__PURE__*/ Vue.extend({ }) } }, - // - // Show and Hide handlers - // + // --- Show/Hide handlers --- + // Show the tooltip show() { - // Show the tooltip const target = this.getTarget() - if ( !target || !contains(document.body, target) || @@ -390,38 +372,29 @@ export const BVTooltip = /*#__PURE__*/ Vue.extend({ // we exit without showing return } - + // If tip already exists, exit early if (this.$_tip || this.localShow) { - // If tip already exists, exit early /* istanbul ignore next */ return } - // In the process of showing this.localShow = true - // Create a cancelable BvEvent const showEvt = this.buildEvent('show', { cancelable: true }) this.emitEvent(showEvt) + // Don't show if event cancelled /* istanbul ignore next: ignore for now */ if (showEvt.defaultPrevented) { - // Don't show if event cancelled // Destroy the template (if for some reason it was created) /* istanbul ignore next */ this.destroyTemplate() - // Clear the localShow flag - /* istanbul ignore next */ - this.localShow = false /* istanbul ignore next */ return } - // Fix the title attribute on target this.fixTitle() - // Set aria-describedby on target this.addAriaDescribedby() - // Create and show the tooltip this.createTemplateAndShow() }, @@ -473,49 +446,42 @@ export const BVTooltip = /*#__PURE__*/ Vue.extend({ enable() { this.$_enabled = true // Create a non-cancelable BvEvent - this.emitEvent(this.buildEvent('enabled', {})) + this.emitEvent(this.buildEvent('enabled')) }, disable() { this.$_enabled = false // Create a non-cancelable BvEvent - this.emitEvent(this.buildEvent('disabled', {})) + this.emitEvent(this.buildEvent('disabled')) }, - // - // Handlers for template events - // + // --- Handlers for template events --- + // When template is inserted into DOM, but not yet shown onTemplateShow() { - // When template is inserted into DOM, but not yet shown // Enable while open listeners/watchers this.setWhileOpenListeners(true) }, + // When template show transition completes onTemplateShown() { - // When template show transition completes const prevHoverState = this.$_hoverState this.$_hoverState = '' if (prevHoverState === 'out') { this.leave(null) } // Emit a non-cancelable BvEvent 'shown' - this.emitEvent(this.buildEvent('shown', {})) + this.emitEvent(this.buildEvent('shown')) }, + // When template is starting to hide onTemplateHide() { - // When template is starting to hide // Disable while open listeners/watchers this.setWhileOpenListeners(false) }, + // When template has completed closing (just before it self destructs) onTemplateHidden() { - // When template has completed closing (just before it self destructs) - // TODO: - // The next two lines could be moved into `destroyTemplate()` - this.removeAriaDescribedby() - this.restoreTitle() + // Destroy the template this.destroyTemplate() // Emit a non-cancelable BvEvent 'shown' this.emitEvent(this.buildEvent('hidden', {})) }, - // - // Utility methods - // + // --- Utility methods --- getTarget() { // Handle case where target may be a component ref let target = this.target ? this.target.$el || this.target : null @@ -575,6 +541,18 @@ export const BVTooltip = /*#__PURE__*/ Vue.extend({ const target = this.getTarget() return this.isDropdown() && target && select(DROPDOWN_OPEN_SELECTOR, target) }, + clearHoverTimeout() { + if (this.$_hoverTimeout) { + clearTimeout(this.$_hoverTimeout) + this.$_hoverTimeout = null + } + }, + clearVisibilityInterval() { + if (this.$_visibleInterval) { + clearInterval(this.$_visibleInterval) + this.$_visibleInterval = null + } + }, clearActiveTriggers() { for (const trigger in this.activeTrigger) { this.activeTrigger[trigger] = false @@ -628,9 +606,7 @@ export const BVTooltip = /*#__PURE__*/ Vue.extend({ removeAttr(target, 'data-original-title') } }, - // - // BvEvent helpers - // + // --- BvEvent helpers --- buildEvent(type, opts = {}) { // Defaults to a non-cancellable event return new BvEvent(type, { @@ -653,9 +629,7 @@ export const BVTooltip = /*#__PURE__*/ Vue.extend({ } this.$emit(evtName, bvEvt) }, - // - // Event handler setup methods - // + // --- Event handler setup methods --- listen() { // Enable trigger event handlers const el = this.getTarget() @@ -663,10 +637,8 @@ export const BVTooltip = /*#__PURE__*/ Vue.extend({ /* istanbul ignore next */ return } - // Listen for global show/hide events this.setRootListener(true) - // Set up our listeners on the target trigger element this.computedTriggers.forEach(trigger => { if (trigger === 'click') { @@ -770,9 +742,7 @@ export const BVTooltip = /*#__PURE__*/ Vue.extend({ target.__vue__[on ? '$on' : '$off']('shown', this.forceHide) } }, - // - // Event handlers - // + // --- Event handlers --- handleEvent(evt) { // General trigger event handler // target is the trigger element From 0c19aeae3575684a4d717e06f05f7af5117df112 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jacob=20M=C3=BCller?= Date: Tue, 19 Nov 2019 09:20:02 +0100 Subject: [PATCH 5/7] Update bv-tooltip.js --- src/components/tooltip/helpers/bv-tooltip.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/components/tooltip/helpers/bv-tooltip.js b/src/components/tooltip/helpers/bv-tooltip.js index 12ced0fe3f6..c714db57ea9 100644 --- a/src/components/tooltip/helpers/bv-tooltip.js +++ b/src/components/tooltip/helpers/bv-tooltip.js @@ -326,6 +326,7 @@ export const BVTooltip = /*#__PURE__*/ Vue.extend({ }, // Destroy the template instance and reset state destroyTemplate() { + this.setWhileOpenListeners(false) this.clearHoverTimeout() this.$_hoverState = '' this.clearActiveTriggers() From 1d0712f02f5f9d42aca4489ed6c435547026ef83 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jacob=20M=C3=BCller?= Date: Tue, 19 Nov 2019 09:23:02 +0100 Subject: [PATCH 6/7] Update bv-tooltip.js --- src/components/tooltip/helpers/bv-tooltip.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/components/tooltip/helpers/bv-tooltip.js b/src/components/tooltip/helpers/bv-tooltip.js index c714db57ea9..b21dab37e9b 100644 --- a/src/components/tooltip/helpers/bv-tooltip.js +++ b/src/components/tooltip/helpers/bv-tooltip.js @@ -36,7 +36,6 @@ import { keys } from '../../../utils/object' import { warn } from '../../../utils/warn' import { BvEvent } from '../../../utils/bv-event.class' import { BVTooltipTemplate } from './bv-tooltip-template' -import { template } from 'handlebars' const NAME = 'BVTooltip' From e82b3946332f46629aa1b5eab843f0438102cb95 Mon Sep 17 00:00:00 2001 From: Troy Morehouse Date: Tue, 19 Nov 2019 16:45:44 -0400 Subject: [PATCH 7/7] Update bv-tooltip.js --- src/components/tooltip/helpers/bv-tooltip.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/tooltip/helpers/bv-tooltip.js b/src/components/tooltip/helpers/bv-tooltip.js index b21dab37e9b..f0cfe6aa72b 100644 --- a/src/components/tooltip/helpers/bv-tooltip.js +++ b/src/components/tooltip/helpers/bv-tooltip.js @@ -203,7 +203,7 @@ export const BVTooltip = /*#__PURE__*/ Vue.extend({ this.$_hoverState = '' this.$_visibleInterval = null this.$_enabled = !this.disabled - this.$_noop = noop + this.$_noop = noop.bind(this) // Destroy ourselves when the parent is destroyed if (this.$parent) {