Skip to content
This repository has been archived by the owner on Apr 18, 2023. It is now read-only.

Stale song-change notifications #14

Open
derat opened this issue Apr 16, 2021 · 5 comments
Open

Stale song-change notifications #14

derat opened this issue Apr 16, 2021 · 5 comments
Assignees
Labels
bug Something isn't working

Comments

@derat
Copy link
Owner

derat commented Apr 16, 2021

Old song-change notifications sometimes don't get cleared. One cause might be closing or reloading the page while the notification is displayed.

@derat derat added the bug Something isn't working label Apr 16, 2021
@derat derat self-assigned this Apr 16, 2021
@derat
Copy link
Owner Author

derat commented Apr 21, 2021

I think I've seen this other times without reloading the page.

derat added a commit that referenced this issue Apr 21, 2021
Clean up ugly code for managing song-change notifications.
I'm not sure where the bug is, so I'm not sure if this will
fix it.
@derat
Copy link
Owner Author

derat commented May 6, 2021

I think that this still happens occasionally, but I'm not sure of the cause. It might be a Chrome OS bug where notifications aren't getting closed while the screen is locked.

@derat
Copy link
Owner Author

derat commented Sep 12, 2021

This still happens all the time. It's unrelated to screen-locking. I should investigate whether it happens when I foreground the tab while the notification is being shown.

@derat
Copy link
Owner Author

derat commented Sep 17, 2021

I'm seeing this happen even when the tab has been in the background the whole time.

Current best theory: Sometimes notifications seem to take a while to show up (I have no idea why). Maybe the delay is sometimes longer than the 3-second timeout to close the notification. The code in music-player.js looks reasonable to me, but maybe Chrome has a bug where close() doesn't work if the notification hasn't been shown yet:

      this.notification_ = new Notification(
        `${song.artist}\n${song.title}`,
        options
      );
      this.closeNotificationTimeoutId_ = window.setTimeout(() => {
        this.closeNotificationTimeoutId_ = null;
        this.closeNotification_();
      }, this.constructor.NOTIFICATION_SEC_ * 1000);

@derat
Copy link
Owner Author

derat commented Sep 17, 2021

Just saw a notification show up a full minute after a song had started playing (and then not get closed automatically). My best guess now is that Chrome is waiting for the notification's image to be downloaded before displaying anything.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant