Fix: setMediaElement to properly subscribe/unsubscribe media events (#…

…3278)

* Set 'setMediaElement' as protected in Player

* Handle player events subscribe/unsubsribe when setting media
- First we unsubscribe from all existing media event listeners
- Then after setting new media element we subscribe to media events

* Add additional specs for setMediaElement

cypress/e2e/basic.cy.js

@@ -177,13 +177,80 @@ describe('WaveSurfer basic tests', () => {
     })
   })
 
-  it('should set media without errors', () => {
-    cy.window().then((win) => {
-      const media = document.createElement('audio')
-      media.id = 'new-media'
-      win.wavesurfer.setMediaElement(media)
-      expect(win.wavesurfer.getMediaElement().id).to.equal('new-media')
+  describe('setMediaElement', () => {
+    const MEDIA_EVENTS = ['timeupdate', 'play', 'pause', 'emptied', 'ended', 'seeking']
+    let orignalMedia
+
+    // Mock add/remove event listeners for `media` elements
+    const attachMockListeners = (el) => {
+      el.eventListenerList = {}
+      el.addEventListener = (eventName, callback, options) => {
+        if (!el.eventListenerList[eventName]) el.eventListenerList[eventName] = [];
+        el.eventListenerList[eventName].push(callback);
+      };
+
+      el.removeEventListener = (eventName, callback) => {
+        if (el.eventListenerList[eventName]) delete el.eventListenerList[eventName]
+      }
+    }
+
+    beforeEach((done) => {
+      cy.window().then((win) => {
+        win.wavesurfer.destroy()
+
+        orignalMedia = document.createElement('audio')
+        attachMockListeners(orignalMedia)
+
+        const waitForReady = new Promise((resolve) => {
+          win.wavesurfer = win.WaveSurfer.create({
+            container: '#waveform',
+            url: '../../examples/audio/demo.wav',
+            media: orignalMedia
+          })
+
+          win.wavesurfer.once('ready', () => resolve())
+        })
+
+        cy.wrap(waitForReady).then(done)
+      })
+    })
+
+    it('should set media without errors', () => {
+      cy.window().then((win) => {
+        const media = document.createElement('audio')
+        media.id = 'new-media'
+        win.wavesurfer.setMediaElement(media)
+        expect(win.wavesurfer.getMediaElement().id).to.equal('new-media')
+      })
     })
+
+    it('should unsubscribe events from removed media element', () => {
+      cy.window().then((win) => {
+        const media = document.createElement('audio')
+
+        MEDIA_EVENTS.forEach((event) => {
+          expect(orignalMedia.eventListenerList[event]).to.exist
+          expect(orignalMedia.eventListenerList[event].length).to.equal(1)
+        })
+
+        win.wavesurfer.setMediaElement(media)
+        expect(orignalMedia.eventListenerList).to.be.empty
+      })
+    })
+
+    it('should subscribe events for newly set media element', () => {
+      cy.window().then((win) => {
+        const newMedia = document.createElement('audio')
+        attachMockListeners(newMedia)
+
+        win.wavesurfer.setMediaElement(newMedia)
+        MEDIA_EVENTS.forEach((event) => {
+          expect(newMedia.eventListenerList[event]).to.exist
+          expect(newMedia.eventListenerList[event].length).to.equal(1)
+        })
+      })
+    })
+
   })
 
   it('should return true when calling isPlaying() after play()', (done) => {

src/player.ts

@@ -83,6 +83,10 @@ class Player<T extends GeneralEventTypes> extends EventEmitter<T> {
     this.media.load()
   }
 
+  protected setMediaElement(element: HTMLMediaElement) {
+    this.media = element
+  }
+
   /** Start playing the audio */
   public play(): Promise<void> {
     return this.media.play()
@@ -152,11 +156,6 @@ class Player<T extends GeneralEventTypes> extends EventEmitter<T> {
     return this.media
   }
 
-  /** Set HTML media element */
-  public setMediaElement(element: HTMLMediaElement) {
-    this.media = element
-  }
-
   /** Set a sink id to change the audio output device */
   public setSinkId(sinkId: string): Promise<void> {
     // See https://developer.mozilla.org/en-US/docs/Web/API/HTMLMediaElement/setSinkId

src/wavesurfer.ts

@@ -131,6 +131,7 @@ class WaveSurfer extends Player<WaveSurferEvents> {
   private plugins: GenericPlugin[] = []
   private decodedData: AudioBuffer | null = null
   protected subscriptions: Array<() => void> = []
+  protected mediaSubscriptions: Array<() => void> = []
 
   /** Create a new WaveSurfer instance */
   public static create(options: WaveSurferOptions) {
@@ -177,7 +178,7 @@ class WaveSurfer extends Player<WaveSurferEvents> {
   }
 
   private initPlayerEvents() {
-    this.subscriptions.push(
+    this.mediaSubscriptions.push(
       this.onMediaEvent('timeupdate', () => {
         const currentTime = this.getCurrentTime()
         this.renderer.renderProgress(currentTime / this.getDuration(), this.isPlaying())
@@ -270,6 +271,11 @@ class WaveSurfer extends Player<WaveSurferEvents> {
     })
   }
 
+  private unsubscribePlayerEvents() {
+    this.mediaSubscriptions.forEach((unsubscribe) => unsubscribe())
+    this.mediaSubscriptions = []
+  }
+
   /** Set new wavesurfer options and re-render it */
   public setOptions(options: Partial<WaveSurferOptions>) {
     this.options = Object.assign({}, this.options, options)
@@ -443,11 +449,19 @@ class WaveSurfer extends Player<WaveSurferEvents> {
     this.load('', [[0]], 0.001)
   }
 
+  /** Set HTML media element */
+  public setMediaElement(element: HTMLMediaElement) {
+    this.unsubscribePlayerEvents()
+    super.setMediaElement(element)
+    this.initPlayerEvents()
+  }
+
   /** Unmount wavesurfer */
   public destroy() {
     this.emit('destroy')
     this.plugins.forEach((plugin) => plugin.destroy())
     this.subscriptions.forEach((unsubscribe) => unsubscribe())
+    this.unsubscribePlayerEvents()
     this.timer.destroy()
     this.renderer.destroy()
     super.destroy()