sdefresne / gerrit-monitor

Source for Gerrit Monitor Chrome extension
Apache License 2.0
21 stars 16 forks source link

Ensure notification click callback remains installed. #26

Closed davidgreenaway closed 4 years ago

davidgreenaway commented 4 years ago

We currently attempt to register a notification click handler during extension startup. At some point, however, the event handler is lost prevent notification clicks from having any effect.

Use a more direct means to determine if a notification handler is available, ensuring that clicks are received corrected.

davidgreenaway commented 4 years ago

Alas, this doesn't actually seem to solve the problem: I am still seeing the handler failing to trigger if the notification is on screen long enough for the extension to be unloaded.

I think the most robust solution might be to just persist the extension. I'll give it a try and update this PR.

davidgreenaway commented 4 years ago

Making the extension's background page persistent appears to resolve the issue with notification callback registrations being lost, and hence notifications not being able to be clicked on if they are on-screen for too long.

It does mean that the extension will now have to persist in memory, but I don't have a better solution for the moment.

sdefresne commented 4 years ago

Thank you for the fix.

I don't think there is any issue with making the background page persistent. I made it non-persistent because it was recommended in the tutorial but it does not mention whether this is supposed to work or not with notifications.