mbin icon indicating copy to clipboard operation
mbin copied to clipboard

using mercure somehow breaks bfcache

Open asdfzdfj opened this issue 1 year ago • 8 comments

in short: mercure + bfcache + ~~onbeforeunload~~ => page cannot be stored in bfcache, causing every navigation to do a page load

possible solution: rewrite teardown function to either use pagehide event or stimulusjs disconnect() function

note that I might be off here, if anyone knows what's more likely them I'm all ears

EDIT 1: ok, maybe onbeforeunload isn't tha main culprit, it seems like having an open connection to mercure at all breaks bfcache, still needs more investigating


based on discussion/discovery here

You know what's great... It is mercure... I had it enabled and it is disabled on kbin.run Now I have disabled it and no more extra requests...

Originally posted by @BentiGorlich in https://github.com/MbinOrg/mbin/issues/689#issuecomment-2045195482

merde*, I think I have idea why:

  • this looks like a browser bfcache territory
  • the notification controller sets up onbeforeunload event listener to tear down the mercure connection https://github.com/MbinOrg/mbin/blob/3e68837b7c06310d5978bbc6190876ae08fd461b/assets/controllers/notifications_controller.js#L13-L21
  • this firefox docs says that the page is ineligible if beforeunload listener is used (the docs seems ancient but that's the only page of firefox bfcache docs I could find atm)

* somehow this sound came up in my head upon reading that

Originally posted by @asdfzdfj in https://github.com/MbinOrg/mbin/issues/689#issuecomment-2045229002

asdfzdfj avatar Apr 09 '24 13:04 asdfzdfj

I just tried on my server changing window.onbeforeunload to window.onpagehide and now it works properly even on Fennec

BentiGorlich avatar Apr 09 '24 14:04 BentiGorlich

on my own testing it's still some bit inconsistent, you sure that you didn't forgot to enable mercure back when testing?

asdfzdfj avatar Apr 09 '24 14:04 asdfzdfj

I am very sure that mercure was deactivated when testing, however Fennec still had the problems, because the notifications controller is always loaded, even when mercure is disabled

BentiGorlich avatar Apr 09 '24 14:04 BentiGorlich

But enabling mercure still breaks the cache, that's for sure :grin:

BentiGorlich avatar Apr 09 '24 14:04 BentiGorlich

yeah that seems to be it, it looks like the other half/part seems to be having mercure connection opened at all, so that's more to investigate

asdfzdfj avatar Apr 09 '24 14:04 asdfzdfj

I just tried enabling mercure, but have it not be reachable, so the request fails instantly. That solves the issue. But I think our code behaves differently when the connection is established versus not established... It is really a bummer that it doesn't just use web sockets, but this weird "eventsource" call...

BentiGorlich avatar Apr 09 '24 14:04 BentiGorlich

It is really a bummer that it doesn't just use web sockets, but this weird "eventsource" call...

Maybe I am wrong about it though. Seems like eventsources are way older and have wider support than websockets... Still weird

BentiGorlich avatar Apr 09 '24 15:04 BentiGorlich

This issue is stale because it has been open 50 days with no activity. Remove stale label or comment or this will be closed in 6 days.

github-actions[bot] avatar May 30 '24 02:05 github-actions[bot]