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

settings: fix for default web browser #16

Conversation

sergio-costas
Copy link

@sergio-costas sergio-costas commented Jun 20, 2023

This fix allows to set and check the default web browser, thus removing the nasty message that Firefox isn't your default browser.
Thanks for helping us make a better snapd!
Have you signed the license agreement and read the contribution guide?

@sergio-costas sergio-costas marked this pull request as draft June 20, 2023 12:44
@sergio-costas sergio-costas force-pushed the UDENG-635-core-desktop-fix-url-handling-default-browser branch from 3371e9a to 47cd4b9 Compare June 20, 2023 17:47
@sergio-costas sergio-costas marked this pull request as ready for review June 20, 2023 17:47
@sergio-costas sergio-costas changed the title Quick'n'dirty fix for default web browser Fix for default web browser Jun 20, 2023
@sergio-costas sergio-costas changed the title Fix for default web browser settings: fix for default web browser Jun 20, 2023
@sergio-costas sergio-costas force-pushed the UDENG-635-core-desktop-fix-url-handling-default-browser branch 2 times, most recently from 66989fb to 3eae3f0 Compare June 21, 2023 13:40
@codecov
Copy link

codecov bot commented Jun 22, 2023

Codecov Report

Merging #16 (fcba778) into master (0da6ea8) will decrease coverage by 0.04%.
The diff coverage is 25.00%.

❗ Current head fcba778 differs from pull request most recent head bc7b34e. Consider uploading reports for the commit bc7b34e to get more accurate results

@@            Coverage Diff             @@
##           master      #16      +/-   ##
==========================================
- Coverage   78.61%   78.57%   -0.04%     
==========================================
  Files         992      992              
  Lines      122905   122935      +30     
==========================================
- Hits        96620    96597      -23     
- Misses      20210    20250      +40     
- Partials     6075     6088      +13     
Flag Coverage Δ
unittests 78.57% <25.00%> (-0.04%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
interfaces/builtin/desktop.go 91.42% <ø> (ø)
usersession/userd/settings.go 54.80% <25.00%> (-18.23%) ⬇️

... and 6 files with indirect coverage changes

@sergio-costas sergio-costas force-pushed the UDENG-635-core-desktop-fix-url-handling-default-browser branch from 560744a to fcba778 Compare June 23, 2023 10:53
Copy link
Collaborator

@jhenstridge jhenstridge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this could be simplified a bit by dropping the xdg-open and xdg-settings proxy scripts from core22-desktop and replacing them with the xdg-utils versions. I don't think there is anything in ubuntu-desktop-session that needs the proxies, and this would simplify the snapd changes here, since snap userd could just rely on the scripts in /usr/bin.

With luck, this would also fix snaps trying to use the xdg-open proxy...

build-aux/snap/snapcraft.yaml Outdated Show resolved Hide resolved
interfaces/builtin/desktop.go Outdated Show resolved Hide resolved
interfaces/builtin/desktop.go Outdated Show resolved Hide resolved
usersession/userd/settings.go Outdated Show resolved Hide resolved
@sergio-costas sergio-costas force-pushed the UDENG-635-core-desktop-fix-url-handling-default-browser branch from 5a11dee to 148e991 Compare July 5, 2023 17:30
@kenvandine
Copy link
Collaborator

gnome-control-center might call xdg-settings in the default apps panel.

@jhenstridge
Copy link
Collaborator

gnome-control-center might call xdg-settings in the default apps panel.

It doesn't. It uses the g_app_info_set_as_default_for_type glib API, which makes equivalent changes to $XDG_CONFIG_HOME/mimeapps.list directly.

This fix allows to set and check the default web browser, thus
removing the nasty message that Firefox isn't your default
browser.

It basically copies the original xdg-settings and xdg-mime
scripts inside snapd, and calls the former with the full path.
Also, the former has been modified to call the xdg-mime
script located inside snapd.
@sergio-costas sergio-costas force-pushed the UDENG-635-core-desktop-fix-url-handling-default-browser branch from f543041 to 6c0ae5a Compare July 11, 2023 11:25
sergio-costas added a commit to sergio-costas/core-base-desktop that referenced this pull request Jul 12, 2023
This patch complements canonical/ubuntu-core-desktop-snapd#16
to allow to set the default web browser in core desktop.
@sergio-costas sergio-costas force-pushed the UDENG-635-core-desktop-fix-url-handling-default-browser branch from 81b1254 to bc7b34e Compare July 13, 2023 18:39
Copy link
Collaborator

@kenvandine kenvandine left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me now

@kenvandine kenvandine merged commit bcf1f66 into canonical:master Jul 14, 2023
10 of 20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants