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

element-desktop: set dbus default for firefox #123777

Merged
merged 1 commit into from
May 20, 2021

Conversation

evils
Copy link
Member

@evils evils commented May 20, 2021

Motivation for this change

allows me to open links in element-desktop in firefox-wayland on sway

until a solution based on #57602 (comment) is implemented in nixos, this seems to be the only way to fix this that doesn't involve every wayland user adding a fix for the issue

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
    • /nix/store/0yjnyhqaqhvxxzqazziz9w6kxwmmh05v-element-desktop-1.7.28 1219371984
    • /nix/store/bb8pl84j5gfrhwy7z2bh25msdv3i35hw-element-desktop-1.7.28 1219372032
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

fixes opening links in firefox-wayland on wayland
Copy link
Member

@thufschmitt thufschmitt left a comment

Choose a reason for hiding this comment

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

I can confirm that this fixed it for me. And it’s a transparent and non-invasive change, so 👍

@r-rmcgibbo
Copy link

Result of nixpkgs-review pr 123777 at becc715 run on x86_64-linux 1

2 packages built successfully:
  • element-desktop
  • tests.trivial
3 suggestions:
  • warning: build-tools-in-build-inputs

    rsync is a build tool so it likely goes to nativeBuildInputs, not buildInputs.

    Near pkgs/development/tools/yarn2nix-moretea/yarn2nix/default.nix:292:7:

        |
    292 |       buildInputs = [ yarn nodejs rsync ] ++ extraBuildInputs;
        |       ^
    
  • warning: missing-phase-hooks

    installPhase should probably contain runHook preInstall and runHook postInstall.

    Near pkgs/development/tools/yarn2nix-moretea/yarn2nix/default.nix:332:7:

        |
    332 |       installPhase = attrs.installPhase or ''
        |       ^
    
  • warning: build-tools-in-build-inputs

    yarn is a build tool so it likely goes to nativeBuildInputs, not buildInputs.

    Near pkgs/development/tools/yarn2nix-moretea/yarn2nix/default.nix:292:7:

        |
    292 |       buildInputs = [ yarn nodejs rsync ] ++ extraBuildInputs;
        |       ^
    

@evils
Copy link
Member Author

evils commented May 26, 2021

seems like @buckley310 on #120228 found this actually breaks opening links for them

can you confirm you meant running element-desktop without the wrapper works for you on X11
and this was with firefox? (asking because your avatar is iceweasel's logo…)

@evils evils mentioned this pull request May 26, 2021
22 tasks
@buckley310
Copy link
Contributor

Yep, I'm using firefox on X11. When i launch element manually the same way as the wrapper script, just without the MOZ_DBUS_REMOTE variable, everything works fine. When element is launched with that variable set, and i already have a browser running, clicking links results in the "Firefox is already running, but is not responding..." error.

@evils
Copy link
Member Author

evils commented May 27, 2021

i was assuming firefox on X11 listened on dbus as well...

i'm assuming we have more X11 users, so this should probably be reverted
and this change made it into the 21.05 branch so it'll need a backport

wayland users can still set the MOZ_DBUS_REMOTE environmental variable before calling element-desktop, or try the workaround mentioned in the 57602 issue comment linked in the initial comment here

@buckley310
Copy link
Contributor

Is there any downside in having MOZ_DBUS_REMOTE=1 always? Like what if that was added to the firefox wrapper, rather than the element wrapper?

@symphorien
Copy link
Member

Ok, so if we revert this we break wayland and if we keep it we break x11?

@evils
Copy link
Member Author

evils commented May 27, 2021

Ok, so if we revert this we break wayland and if we keep it we break x11?

seems like it
though i don't currently know of a way to make it work on X11 with this patch
while it can be made to work on wayland without this patch

so i'm going to open a revert PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants