search

pop-os / launcher

Modular IPC-based desktop launcher service
Mozilla Public License 2.0
232 stars 48 forks source link

Add generic `www` pattern to open any website #156

Closed canadaduane closed 1 year ago

canadaduane commented 1 year ago

Opens a website in default browser, e.g. www github.com

canadaduane commented 1 year ago

No matter what browser is set to default (e.g., midori, firefox, chrome, brave, chromium) it only seems to launch Firefox.

Oh interesting! I think I saw this at first as well, but didn't understand what was going on--so I then logged out and back, and it worked for me. It now launches correctly in Chromium (my system browser).

Are you seeing anything similar? Or does it only try to launch Firefox, even after logout?

canadaduane commented 1 year ago

Also, the icon is a bit strange--but perhaps this is normal for a URL that it does not know?

image

n3m0-22 commented 1 year ago

I have tested with Logout, Reboot, Poweroff/Restart, Alt+ F2 and r, and nothing seems to get it working on my end. As for the icon this PR https://github.com/pop-os/launcher/pull/129 may be useful. To be sure I tested on a few other machines one of which is a clean install, but still no luck.

canadaduane commented 1 year ago

I have tested with Logout, Reboot, Poweroff/Restart, Alt+ F2 and r, and nothing seems to get it working on my end... To be sure I tested on a few other machines one of which is a clean install, but still no luck.

Thanks for checking. That's very thorough.

I wonder if it could be because I have already merged and installed PR #155 that it is working for me? Could that explain the difference we're seeing?

As for the icon this PR https://github.com/pop-os/launcher/pull/129 may be useful.

Thanks, I didn't realize the icon could be set for a URL. I'll add this in to the PR.

n3m0-22 commented 1 year ago

The first machine I tested on had #155 installed and I updated from there. I didn't seem to make any difference. I did try with and without #155 JIC.

canadaduane commented 1 year ago

Ok, I think I've figured out what's going on. There is a "panic" that causes the launcher to crash when attempting to retrieve an icon URL without a domain (i.e. http:// with no domain).

Jan 12 19:44:02 rosie gnome-shell[184724]: favicon_path: /home/duane/.cache/pop-launcher/Open Website.ico
Jan 12 19:44:02 rosie gnome-shell[184724]: thread 'main' panicked at 'invalid url: EmptyHost', /home/duane/tmp/launcher/plugins/src/web/mod.rs:141:36
Jan 12 19:44:02 rosie gnome-shell[184724]: note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Somehow, I was able to get it to fetch some image (I'm not sure how!) and it cached it in ~/.cache/pop-launcher/Open Website.ico. From then on, the www pattern to open a website worked correctly, albeit with a garbled icon image.

n3m0-22 commented 1 year ago

I went back to square one with this. I setup a fresh install then built and installed #155 and this PR. After that it worked. I then tested on another machine with the staging repo to get #155 and then added this PR. That also worked. To confirm I tested on a old VM and did the same updates. That worked as well. I'm not sure why it was not working before, but I can confidently say this is working now on my end.

@canadaduane Before I approve do you want to push the changes with the icon?

canadaduane commented 1 year ago

Thanks for testing this again. After discovering #157 I think we are hitting a "random chance" bug that is affecting our success or failure here. It could have just been random chance that it worked for you, because it would depend on whether or not an image had enough time to cache asynchronously before panicking. So I would recommend waiting for that PR merge first.

canadaduane commented 1 year ago

@canadaduane Before I approve do you want to push the changes with the icon?

I tried to get an icon working, but failed :S I could just be doing it wrong. Do we have an example config.ron somewhere with an icon in use for a web URL?

n3m0-22 commented 1 year ago

This example from #129 is working for me.

(
   matches: ["np"],
   queries: [(
      name: "Nix Packages", 
      query: "search.nixos.org/packages?channel=unstable&query=",
      icon: "https://nixos.wiki/nixos-logo-small.png",
   )]
),

If you are talking about using a local icon like something in /usr/share/icons I have not tested that as it was not the goal of the PR, but I can look into it.

As for #157 I'll start checking it, and keep an eye out to test any changes as they come in.

mmstick commented 1 year ago

@canadaduane Could you include the commits from #157 in this PR? This PR relies on that PR so it'd make sense to have them in a single PR so QA only has to test one PR.

canadaduane commented 1 year ago

PRs merged into this one for easier QA. Thanks for the suggestion!

canadaduane commented 1 year ago

"https://nixos.wiki/nixos-logo-small.png"

I don't understand why, but I can't seem to get an icon working alongside this change. It shows the magnifying glass icon, which is fine. There is no accompanying "URL" icon, however. Adding the "icon:" field as @n3m0-22 suggested does not seem to work for now (FWIW I tried "https://upload.wikimedia.org/wikipedia/commons/thumb/7/72/Crystal128-browser.svg/179px-Crystal128-browser.svg.png" as an icon, but it didn't show up when www pattern was given in the launcher).

canadaduane commented 1 year ago

I don't understand why, but I can't seem to get an icon working alongside this change.

Ok, it's my PR's fault. The early return that guards against invalid pattern URLs simultaneously prevents the icon URL from being fetched in the backround. Working on it.

canadaduane commented 1 year ago

Ok, I think this is ready for review again.