MS Edge compatibility issue with processQueue()

[corinnaSchultz]

This is a little hard to reproduce, and I think there is an issue with how Edge handles iframes. We have no problem with this same app in Chrome or Safari or Firefox.

What I have observed is that when my app loads for the first time, in a fresh tab (disabling the cache in the devtools doesn't work, this needs to be a fresh tab), the first event object that is passed into processQueue results in the boolean event.source === global returning false.

I have seen this false value come from two different causes: 1) The event is a "load" event and not a "message" event. In the course of my debugging, I only saw this a couple of times, so I think there may be a race condition in the way the processQueue function is called.

2) Much more common, what I observed is that when the page loaded, the event.source was the "main" window, and global was the iframe window object. I don't know what it is about how my app loads that causes the window objects to be different sometimes, and the same other times. This code seems to be hit three times during load, and only the first time are the window objects different.

Something about the false value is causing the rest of my app not to work, possibly because the queue never gets flushed?

If I change the code so that flushQueue() is called outside the if statement, then things seem to work ok (I am just beginning browser testing with Edge, so it's possible I'll find other issues).

processQueue = (function () {
   return hasPostMessage
    ? function processQueue (event) {
          if (event.source === global && event.data === messageName) {
          event.stopPropagation();
          }
              flushQueue();   // THIS IS THE LINE I MOVED
         }
       : flushQueue;
})()

I am reluctant to submit this as a Pull Request, because I'm not sure if there are any negative consequences to making this change. I'd appreciate it if you/someone could review this.

[scottlow]

I work on the Microsoft Edge team and came here to report this same issue. We have a bug logged on our side to track the investigation of this, but it would be great if a workaround could be implemented in Freezer for the time being.

[arqex]

Hey guys sorry for the delay, I've been out for some days.

This issue is interesting. The message passing here is used because in browsers we don't have a simple way of executing things on the next JS execution loop. By default freezer doesn't emit update events immediately, it allow us to make multiple changes to its data and the event is emmited in the "next tick".

Using timeouts to delay that event triggering is too slow (a timeout of 0ms is not really executing the function on the next tick), so the alternative was using the browser's message system: Freezer emits a message nexttick to the current window, and listen for that message that arrives on the next tick.

If the message is not coming from the same window (event.source === global) don't emit the event. If we remove that condition, we run the risk of triggering events by messages produced by other freezer instances inside of iframes or browser's plugins.

If we move the flushQueue call outside the if block, we are just flushing on every browser message, and that is very often (especially in browsers with many plugins) and may lead to unexpected behaviours.

The right solution would be creating an id for every freezer instance, and check if the message comes from ourselves before flushing the queue. That way we can skip the current window check.

I'll have a look at the problem this evening, but it would be great to fix the bug at MS Edge too.

Thanks for your feedback guys!

[arqex]

I have just published a new version that it's not using event.source anymore to check what's the origin of the message. Let me know if it fixed your issue.

[scottlow]

@arqex That seems to have done the trick for the case we were seeing. Thanks for the quick turnaround here!

[corinnaSchultz]

I was finally able to test this today. It works for me. Thanks for the quick turnaround!

[corinnaSchultz]

@arqex Could you make this an official release? It would make it easier for our npm config to pick it up.