clappr-plugins icon indicating copy to clipboard operation
clappr-plugins copied to clipboard

fix(media_control): remove subscription duplicate

Open ordyakov opened this issue 3 years ago • 3 comments

bindContainerEvents is called twice.

  • inside bindEvents
  • а few lines below (removed)

ordyakov avatar Sep 15 '22 13:09 ordyakov

Be cautious, i think this line may be required, because in bindEvents, the CORE_ACTIVE_CONTAINER_CHANGED event may have been already dispatched (ie: before we listen). And therefore it is why (i think) it is called it after listening event.

Have you been able to reproduce a case where container events are bind twice on active container ?

kslimani avatar Sep 15 '22 14:09 kslimani

  bindEvents() {
    this.stopListening()
    this.listenTo(this.core, Events.CORE_ACTIVE_CONTAINER_CHANGED, this.onActiveContainerChanged)
    this.listenTo(this.core, Events.CORE_MOUSE_MOVE, this.show)
    this.listenTo(this.core, Events.CORE_MOUSE_LEAVE, () => this.hide(this.options.hideMediaControlDelay))
    this.listenTo(this.core, Events.CORE_FULLSCREEN, this.show)
    this.listenTo(this.core, Events.CORE_OPTIONS_CHANGE, this.configure)
    this.listenTo(this.core, Events.CORE_RESIZE, this.playerResize)
    this.bindContainerEvents() // (2) FIRST TIME
  }
onActiveContainerChanged() {
    this.fullScreenOnVideoTagSupported = null
    this.bindEvents() // (1) INSIDE THIS CALL
    this.setInitialVolume()
    this.changeTogglePlay()
    this.bindContainerEvents() // (3) SECOND TIME
    this.settingsUpdate()
    this.container && this.container.trigger(Events.CONTAINER_PLAYBACKDVRSTATECHANGED, this.container.isDvrInUse())
    this.container && this.container.mediaControlDisabled && this.disable()
    this.trigger(Events.MEDIACONTROL_CONTAINERCHANGED)
  }

ordyakov avatar Sep 15 '22 16:09 ordyakov

Be cautious, i think this line may be required, because in bindEvents, the CORE_ACTIVE_CONTAINER_CHANGED event may have been already dispatched (ie: before we listen). And therefore it is why (i think) it is called it after listening event.

Have you been able to reproduce a case where container events are bind twice on active container ?

You can reproduce this behaviour here http://clappr.io/demo/ by adding breakpoint in any container listener. Every listener will be called twice.

Screenshot 2022-09-20 at 12 06 05

ordyakov avatar Sep 20 '22 09:09 ordyakov