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

Gmail Notifications #376

Merged
merged 8 commits into from
Jul 12, 2024
Merged

Conversation

viktor44
Copy link
Collaborator

What is this PR

Fixed Notifications not working #177 for Gmail

@viktor44
Copy link
Collaborator Author

viktor44 commented May 14, 2024

Hey @magne4000 I need your bright ideas again.
Here is a fix for notifications. Checking them in Gmail.
It more or less works (at least from my point of view 😄 ) except one problem - Stations hangs for ~30 sec when we receive first notification after start.
I don't understand why.
Please take a look when you have time.

@magne4000
Copy link
Member

I cannot test that right now, probably during the next few days I'll be able to, but in the mean time, can you explain what you did to "fix" this issue?
It seems to me you just extracted BxNotification into another file, and injected it before the actual preload, is that it?
If so, my guess is that something crashes before require('../../notification-center/webview-preload');, but it should then be visible in Gmail's page devtool.

@viktor44
Copy link
Collaborator Author

viktor44 commented May 15, 2024

I cannot test that right now, probably during the next few days I'll be able to, but in the mean time, can you explain what you did to "fix" this issue?
It seems to me you just extracted BxNotification into another file, and injected it before the actual preload, is that it?
If so, my guess is that something crashes before require('../../notification-center/webview-preload');, but it should then be visible in Gmail's page devtool.

I separated the code because of isolated context.
Preload script is executed in the node context and setup window.bxApi variable which can be used inside Gmail. Preload script now doesn't have an access to page's window object, so you can't override window. Notification here.
Then inject script is ran in Gmail context and override window. Notification. No ipcRenderer here. You can connect with main application only using window.bxApi.

So the algorithm is the same as before. I only changed the implementation.

require('../../notification-center/webview-preload'); is commented and isn't ran at all

@magne4000
Copy link
Member

I noticed that when I reload gmail page, and then trigger a notification, all handlers seems to be called once more. (So after 10 reload, notification is received 11 times in the rest of the app).

@viktor44
Copy link
Collaborator Author

viktor44 commented Jun 2, 2024

Hey @magne4000 have you had a chance to take a look at the problem Stations hangs for ~30 sec when we receive first notification after start.?

@magne4000
Copy link
Member

Hey @magne4000 have you had a chance to take a look at the problem Stations hangs for ~30 sec when we receive first notification after start.?

I did not do much since my last comment, and probably will not dig further into that matter TBH.
It's one of those issue that reminds me why we changed direction back in the days at Station™, and I'm not willing to spend too much time debugging this project anymore.

That being said, if handlers are registered on page refresh, but previous ones are not dropped, that's probably why you have this issue.
If it comes from internal Services system, you can enable DEBUG=service:* to check if lifecycle seems weird. Otherwise it probably comes from somewhere related to the preload lifecycles.

@viktor44 viktor44 marked this pull request as ready for review July 11, 2024 23:27
@viktor44
Copy link
Collaborator Author

@magne4000 It works now!
Please review

@magne4000
Copy link
Member

@viktor44 any idea what actually fixed the issue?
Anyway good job, feel free to merge when you want!

@viktor44
Copy link
Collaborator Author

@viktor44 any idea what actually fixed the issue?

It was a silly bug.

For Gmail we receive icon URL without https: like
//something.google.com/.../icon.png

Then we ask Electron to create image from this URL

resolve(nativeImage.createFromPath(url));

and it blocks main thread for 30 seconds.

@viktor44 viktor44 merged commit 8d377c1 into getstation:main Jul 12, 2024
3 checks passed
@viktor44 viktor44 deleted the feature/notifications branch August 25, 2024 12:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants