Merge pull request #108 from clappr/feature/fix-leaks

Fix leaks

Author: pedrochamberlain

src/components/core/core.js

@@ -115,6 +115,7 @@ export default class Core extends UIObject {
     this.playerError = new PlayerError(options, this)
     this.configureDomRecycler()
     this.firstResize = true
+    this.styleRendered = false
     this.plugins = []
     this.containers = []
     //FIXME fullscreen api sucks
@@ -367,16 +368,24 @@ export default class Core extends UIObject {
   }
 
   appendToParent() {
-    const style = Styler.getStyleFor(CoreStyle.toString(), { baseUrl: this.options.baseUrl })
-    const resetStyle = Styler.getStyleFor(ResetStyle.toString(), { baseUrl: this.options.baseUrl })
-    this.$el.append(style[0])
-    this.options.includeResetStyle && this.$el.append(resetStyle[0])
-
     const hasCoreParent = this.$el.parent() && this.$el.parent().length
     !hasCoreParent && this.$el.appendTo(this.options.parentElement)
   }
 
+  appendStyles() {
+    if (this.styleRendered) return
+
+    const style = Styler.getStyleFor(CoreStyle.toString(), { baseUrl: this.options.baseUrl })
+    this.$el.append(style[0])
+    if (this.options.includeResetStyle) {
+      const resetStyle = Styler.getStyleFor(ResetStyle.toString(), { baseUrl: this.options.baseUrl })
+      this.$el.append(resetStyle[0])
+    }
+    this.styleRendered = true
+  }
+
   render() {
+    this.appendStyles()
     this.options.width = this.options.width || this.$el.width()
     this.options.height = this.options.height || this.$el.height()
     const size = { width: this.options.width, height: this.options.height }

src/components/core/core.test.js

@@ -285,4 +285,38 @@ describe('Core', function() {
       })
     })
   })
+
+  describe('when rendering', () => {
+    beforeEach(() => {
+      this.core = new Core({})
+    })
+
+    test('append default style element', () => {
+      this.core.render()
+
+      expect(this.core.el.children.length).toEqual(1)
+      expect(this.core.el.children[0].tagName).toEqual('STYLE')
+    })
+
+    test('append default and reset style elements with includeResetStyle set', () => {
+      const newOptions = {
+        includeResetStyle: true,
+      }
+      this.core.configure(newOptions)
+
+      this.core.render()
+
+      expect(this.core.el.children.length).toEqual(2)
+      expect(this.core.el.children[0].tagName).toEqual('STYLE')
+      expect(this.core.el.children[1].tagName).toEqual('STYLE')
+    })
+
+    test('does append style elements twice', () => {
+      this.core.render()
+      this.core.render()
+
+      expect(this.core.el.children.length).toEqual(1)
+      expect(this.core.el.children[0].tagName).toEqual('STYLE')
+    })
+  })
 })

src/components/loader/loader.test.js

@@ -18,7 +18,7 @@ describe('Loader', () => {
   describe('checkVersionSupport function', () => {
     let corePlugin, containerPlugin
     beforeEach(() => {
-      corePlugin = CorePlugin.extend({ name: 'core-plugin', supportedVersion: { min: '0.4.0' } })
+      corePlugin = CorePlugin.extend({ name: 'core-plugin', supportedVersion: { min: '0.5.0' } })
       containerPlugin = ContainerPlugin.extend({ name: 'container-plugin', supportedVersion: { min: '0.4.0', max: '9.9.9' } })
     })
 

src/playbacks/html5_video/html5_video.js

@@ -498,6 +498,7 @@ export default class HTML5Video extends Playback {
   destroy() {
     this._destroyed = true
     this.handleTextTrackChange && this.el.textTracks.removeEventListener('change', this.handleTextTrackChange)
+    this.$el.off('contextmenu')
     super.destroy()
     this.el.removeAttribute('src')
     this.el.load() // load with no src to stop loading of the previous source and avoid leaks