Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Posts double-reblog after extension update/restart #1597

Closed
marcustyphoon opened this issue Aug 28, 2024 · 2 comments · Fixed by #1601
Closed

Posts double-reblog after extension update/restart #1597

marcustyphoon opened this issue Aug 28, 2024 · 2 comments · Fixed by #1601
Assignees
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@marcustyphoon
Copy link
Collaborator

marcustyphoon commented Aug 28, 2024

Platform

macOS 14.4.1 arm64

Browser

Firefox 129.0b9

Addon version

1.0.2

Details

I hate Firefox's extension update method so much. You have no idea.

Sorry, anyway, right, the bug report. We add an event listener on xkitinjectionrequest to handle injected functions in the "main world" context when the extension boots; on Firefox this occurs when you toggle the extension off-and-on or when it gets autoupdated silently in the background. Firefox invalidates old event listeners from the previous invocation of the extension when this happens... but not this one. Therefore, both versions of the listener exist in this case.

All injected functions are therefore run twice (or n times, if you simply toggle the extension off and on n times) each time they're supposed to be run; this includes things like quick reblog's reblog of a post.

This is, obviously and notably, the opposite of #1551, and I don't understand this; the fact that I had to implement that fix sort of implies that this shouldn't be a problem; looking at it another way, the fact that that fix causes this problem implies that I shouldn't have had to implement that fix?

But, like, whatever. Presumably we should put a data attribute on the main world script element with a random string and append that to the relevant event names. (Or set a property on window to a random value and don't respond to events if that value has changed, or something; that doesn't work if the pre-update version does not have that code, so one would have to do some other things as well—there may be better #1538-compatible solutions, but that's what I came up with off the dome).

@marcustyphoon
Copy link
Collaborator Author

I guess another way would be to have the main world script:

  • if window.onInjectionRequest is set, call document.documentElement.removeEventListener with it
  • set window.onInjectionRequest to the new handler
  • call document.documentElement.addEventListener with the new handler

but that exposes the handler to the page code, which is pretty silly.

@marcustyphoon
Copy link
Collaborator Author

I would love if there was a way to just have the event listener itself cancel the rest of the identical listeners (stopImmediatePropagation?), but apparently listeners are run in registration order and we want the opposite of that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment