Undefined Removal Order and Behavior caused by XF.off

[pegasus]

Affected version

2.3.4

When using the Javascript method XF.off to remove an event listener from a known element, the method removes a matching listener from XF.eventHandlers even though the evaluated listener may actually belong to a different element, usually when multiple elements share the same listener. At first glance this may seem safe, because there are still the expected number of listeners in the cache and all listeners can still be removed individually, if in an unexpected order.

But in practice, we tend to only remove listeners from a specific element under specific conditions (such as clicking a button, toggling its class, and giving it a different listener). When using XF.on/off in sequences on multiple elements, this leads to situations where some elements do not successfully have their listeners removed, resulting in ever-increasing stacks of listeners on the element that were expected to be removed but were not. This leads to undefined (and frustrating) behaviors.

The problem does not occur if XF.off is circumvented and element.removeEventListener is called directly.

The problem appears to be in js/xf/core.js:

                XF.eventHandlers[namespace][event] = XF.eventHandlers[namespace][event].filter(handlerData =>
                {
                    if (!handler || handler === handlerData.handler || handler === handlerData.handler.originalHandler)
                    {
                        element.removeEventListener(event, handlerData.handler, options)
                        return false
                    }
                    return true
                })

I have not seen the problem reoccur since making a small tweak to the above. The situation seems to be resolved when the inner condition is changed to only evaluate when handlerData.element === element.

                XF.eventHandlers[namespace][event] = XF.eventHandlers[namespace][event].filter(handlerData =>
                {
                    if (element === handlerData.element && (!handler || handler === handlerData.handler || handler === handlerData.handler.originalHandler))
                    {
                        element.removeEventListener(event, handlerData.handler, options)
                        return false
                    }
                    return true
                })

This bug was found in 2.3.4, but is also present in js/xf/core.js here on xenforo.com.