Multiple "Qti3Player" instances with multiple PCI message and resize listeners which are not removed when beforeDestroy() lifecycle called

[svarokk]

Not sure if i should post this in qti3-item-player-controller repo, sorry for that.

I am using QTI 3 player controller, normally when using it everything is fine, now I need a print function, i've created new view "PrintRunner" which basically calls multiple Qti3Player instances (each per item count) and renders them, then I can print the results using CTRL+P.

Problem is, the "store" has methods "initializePciMessageListener" and "removePciMessageListener" functions, as the store is global, the variables "pciMessageHandler" and "windowResizeHandler" (which are callbacks for the PCI api) are overriden each time the player is loaded. Now I am having a problem when I go to the "PrintRunner" and it has alot of PCI components, the window listeners "message" and "resize" are being added, but when I go back to my "TestRunner" it only removes the last listener of variables "pciMessageHandler" and "windowResizeHandler" in the removePciMessageListener function and now my PCI in "TestRunner" keep duplicating.

Sorry if it's hard to understand, please let me know if so, i'll try to explain better!

Also if you think I'm doing the "PrintRunner" wrong (by using multiple Qti3Player components to render the xml's) please let me know.

Thanks!

Also how I solved that: (had to interfere with source code btw)

store.js:

  resizeListeners: [],
  messageListeners: [],

  initializePciMessageListener () {
    // Keep a handle on the Window Resize Handler and
    // the PCI PostMessage Handler
    this.pciMessageHandler = this.PciMessageListener.bind(this)
    this.windowResizeHandler = this.WindowResize.bind(this)

    window.addEventListener('message', this.pciMessageHandler)
    window.addEventListener('resize', this.windowResizeHandler)

    this.resizeListeners.push( this.windowResizeHandler );
    this.messageListeners.push( this.pciMessageHandler );
  },

  removePciMessageListener() {
    this.resizeListeners.forEach((listener, index) => {
        window.removeEventListener('resize', listener)
        this.resizeListeners.splice(index, 1);
    });
    this.messageListeners.forEach((listener, index) => {
        window.removeEventListener('message', listener)
        this.messageListeners.splice(index, 1);
    });
  },

EDIT: I'm using vue-router to go from "TestRunner" and "PrintRunner" back and forth, no page reloading

[paulgrudnitski]

Thank you so much for posting this issue, and for your proposed/provided solution.

Yes, I am aware of this problem - and there may be others - when there are multiple players in the DOM. And I do very much appreciate that you are collecting listeners in arrays.

Perhaps a better solution:

Assuming we have two items/players, we would have two stores. Consequently, we could add code in initializePciMessageListener to detect if pciMessageHandler and pciResizeHandler have ever been added to window. We could track this by adding a boolean property to window. If the property does not exist, add the two listeners which would then be global for all players in the DOM. Any subsequent calls to initializePciMessageListener would then check the window property and return before adding further listeners.

The only remaining problem (AFAIK) with this is that we can run into problems identifying exactly which item and pci is sending a message or resize event. The current design is only reporting the PCI's response identifier, not the encapsulating item's identifier. So if two different items share the same PCI response variable, this would result in a collision. The solution for this problem would be to pass the item's identifier into the PCI's iframe, which would then enable the PCI to add that same item identifier to the event object.

Of course, this solution would have the weakness that two exact same items (sharing the same item identifier) would result in collisions. But if that isn't a scenario for you, then my idea should work.

Any reactions to this? or do you have a better idea? or a further improvement?

Again, very pleased that you posted this issue. I look forward to your reply and to further improving the component.

---

EDIT: Ha...already thought of a problem with my proposed design...but I'll await your thoughts.

[svarokk]

Hey!

Sorry for the late answer, weekends haha.

I didn't have much time to analyze the source code, sorry for that! But I think your answer is the best for now, having just one listener (one message and one resize) is the best solution ATM.

As I see the Qti3Player.vue:466 ( store.initializePciMessageListener() ) is the starting point for creating the listeners, we could actually store the listeners in the window variable and then check if they're defined, don't override them.

store.js:

  initializePciMessageListener () {
    // Keep a handle on the Window Resize Handler and
    // the PCI PostMessage Handler
    if( window.pciListeners ) return;

    this.pciMessageHandler = this.PciMessageListener.bind(this)
    this.windowResizeHandler = this.WindowResize.bind(this)

    window.pciListeners = {
      resize: this.windowResizeHandler,
      message: this.pciMessageHandler
    };

    window.addEventListener('message', window.pciListeners.message)
    window.addEventListener('resize', window.pciListeners.resize)
  },

  removePciMessageListener() {
    if (window.pciListeners.message !== null) window.removeEventListener('message', window.pciListeners.message)
    if (window.pciListeners.resize !== null) window.removeEventListener('resize', window.pciListeners.resize)

    window.pciListeners = null;
  },

Then next time Qti3Player component is called, it would not override the existing listeners and then we could remove them safely, basically it's the same as you've suggested by the boolean variable.

My version of the controller uses backend fetch methods for the tests/items, so It's not a problem for me having 2 same identifiers as I can easily handle that from backend and remove duplicates, but I see what you mean, it is an issue.

I'm sorry I couldn't be much of help, I'm pretty much very new to vue so I could be missing other better solutions.