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

[Bug] The 10seconds delay is still here when openning a link with windows notifications activated on Win10 #58

Open
Elihpmap opened this issue Nov 28, 2024 · 9 comments

Comments

@Elihpmap
Copy link
Contributor

As we discuted previously there was an issue on Windows 10 where the notification service was delaying the opening of the link to after the notification disappeared (10 second delay)

A fix was proposed in commit 2b10586caa1e4f90d0949fa845a4364e947e2005 but since the new task is still awaited, the focus still doesn't come back to the BrowserService.LaunchAsync() to launch the URL until it is finished (= until the notification has finished being displayed).

The easiest solution to me (without impacting the await uses since that wasn't a good idea last time ) would be to exchange these two lines in BrowserService.cs (lines 41 and 43)

      await notifier.NotifyAsync($"Opening {name}", $"URL: {url}");

      Process.Start(path, $"{args} \"{uri.OriginalString}\"");

to

      Process.Start(path, $"{args} \"{uri.OriginalString}\"");

      await notifier.NotifyAsync($"Opened {name}", $"URL: {url}");

(and changing the "Opening" to "Opened" to stay accurate)

@nref
Copy link
Owner

nref commented Nov 28, 2024

That's the right fix. I had meant to include a change like that in the referenced commit, but I overlooked it. I will gladly merge in a PR with this proposed change.

@Elihpmap
Copy link
Contributor Author

I closed the PR because this fix brings back the tray icon not dissapearing issue...
I don't really know how to fix it correctly then, do you have any insight on the tray icon issue already? And perhaps the tray icon issue is better than the 10 seconds delay in the meantime ?

@nref
Copy link
Owner

nref commented Nov 29, 2024

I will investigate this in the next day or two. Thanks for your trouble.

@nref
Copy link
Owner

nref commented Nov 29, 2024

Version 0.12.6 is working as expected on Win10 and Win11.

@nref nref closed this as completed Nov 29, 2024
@Elihpmap
Copy link
Contributor Author

Elihpmap commented Nov 29, 2024

Not with notifications enabled though... at least not on my PC?
As far as I understand the code, without the reordering of lines mentionned in the first message of this issue, the 10 seconds delay is expectable. But if I'm wrong and something else is to blame, here's my setup:

  • Windows 10 Pro, Version 22H2, Build 19045.5131
  • my CPU is an AMD Ryzen 7 3700X so I only tested x64 builds (not arm64)
  • I tested a few different config but the current state of my config.ini file that triggers this bug is as follow :
[notify]
# Show a desktop notification when opening a link. Defaults to true
enabled = true

[log]
# Write log entries to a file. Defaults to false
enabled = true
# Defaults to C:\Users\<user>\AppData\Local\BrowseRouter\yyyy-MM-dd.log
file = "E:\Bureau\BrowserRouter\Current used version\BrowseRouter.log"

# Default browser is first in list
# Use `{url}` to specify UWP app browser details TOFIX : https://github.com/nref/BrowseRouter/issues/10
[browsers]
firefox = C:\Program Files\Mozilla Firefox\firefox.exe
edge = C:\Program Files (x86)\Microsoft\Edge\Application\msedge.exe
chrome = C:\Program Files\Google\Chrome\Application\chrome.exe

# Url preferences.
# - Only * is treated as a special character (wildcard).
# - Only domains are matched. Don't include protocols e.g. "https://" or paths e.g. "/some/path?query=value"
# - Beware that subdomains don't match automatically, e.g. "youtube.com = chrome" would not launch Chrome for "www.youtube.com"
#   For that reason, you'll often want a leading "*." e.g. "*.youtube.com". 
#   Note: Don't use "*youtube.com" as that would also match e.g. "notyoutube.com".
[urls]


# Source preferences.
# Only * is treated as a special character (wildcard).
# Matches on window title of application used to open link.
# Applied regardless of any url preference match.
[sources]
*Microsoft Teams* = edge
# Default case. Added automatically
# * = whatever

Thanks for your help too!

@nref
Copy link
Owner

nref commented Nov 29, 2024

You're right, I must have a made a mistake. I tried it again on 0.12.6 and got the 10s delay. Let me see what I can do...

@nref
Copy link
Owner

nref commented Nov 29, 2024

I have a fix, could you try it in the linked PR? Your previous PR was good, but additionally, we now have to catch any exception from Process.Start, otherwise the notification is suppressed.

@Elihpmap
Copy link
Contributor Author

Elihpmap commented Dec 2, 2024

Thank you !
Your fix works perfectly when opening one URL, the issue of the tray icon not diseapearing is no more (I still don't understand where does this issue came from 😆).
But when opening 2 links with one call to the application ("Open multiple URLs" launch profile if you want to test it) the first links open immediatly and then the second one takes 10 seconds... this fix is better than nothing for now though, I think the multiple Url is not a very common usage. I'll look into how we could fix it too if you want, with the new action Action.TryRun() since this seem to not trigger the tray icon issue.

@nref
Copy link
Owner

nref commented Dec 2, 2024

I'll look into the 10s delay on the nonfirst URL

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

No branches or pull requests

2 participants