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.

Sign up for GitHub

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

Jump to bottom

firefox: support for native messaging hosts in wrapper #31572

Merged
merged 1 commit into from Nov 14, 2017
Merged

firefox: support for native messaging hosts in wrapper #31572

merged 1 commit into from Nov 14, 2017

Conversation

roastiek
Copy link
Contributor

Motivation for this change

New web extensions sometimes need native binaries installed to work. However in firefox the search path for these binaries is hardcoded to /usr/lib/mozilla/native-messaging-hosts.
I added small patch that allows overriding that path by new env varibale MOZ_SYSTEM_DIR and enhanced the wrapper. Here is an example integration of chrome-gnome-shell.
This will allows to add existing native messaging hosts (#28882 and #24803) to firefox in similar way like plugins .

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-sandbox in nix.conf on non-NixOS)
  • 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 nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

@eqyiel
Copy link
Contributor

eqyiel commented Nov 12, 2017 •
edited

Thanks for working on this, this will help packages like browserpass (which has a NixOS module) and friends.

@deedrah is MOZ_SYSTEM_DIR documented anywhere or is it a name that you made up?

@eqyiel
Copy link
Contributor

eqyiel commented Nov 12, 2017

My bad, I read the patch. I wonder if this is something that can be achieved using a FHS user env wrapper in order to avoid recompiling?

@7c6f434c
Copy link
Member

@eqyiel not sure if a rebuild is worse than requiring user namespaces on non-NixOS Linux in this case

Given that Firefox 57 (with Big Changes™) is expected this week, if we check the patch with the new version first, most users won't even get an extra rebuild.

@eqyiel
Copy link
Contributor

eqyiel commented Nov 13, 2017

@7c6f434c that's true, I didn't even consider that. In any case I will be happy to be able to use native messaging hosts in NixOS!

@7c6f434c
Copy link
Member

@taku0 any opinion (as recently you are the person who usually updates Firefox)? Also, any opinion about trying to combine this with FF57 update?

@taku0
Copy link
Contributor

taku0 commented Nov 13, 2017

@7c6f434c It looks good to me. I don't care rebuilding, but if it is not desirable, I will make FF57 branch on this branch.
@deedrah If this is merged, would you send it also to the upstream for the sake of firefox-bin?

@taku0
Copy link
Contributor

taku0 commented Nov 13, 2017

Here is a WIP FF57 branch (without this PR): https://github.com/taku0/nixpkgs/tree/firefox-bin-57.0

@7c6f434c
Copy link
Member

@taku0 I think rebuilding is fine if the two rebuilds are close; do as is more convenient. I just meant that all the events would be relatively close to each other.

@7c6f434c
Copy link
Member

Wait a minute, the release source tarball is already available?

@roastiek
Copy link
Contributor Author

Hmm, maybe I have found another way. Most of the code of native messaging is written in JavaScript and theoretically it should be possible to change it in the wrapper. I known where and what to change but I have problems with repacking omni.ja.

@taku0
Copy link
Contributor

taku0 commented Nov 14, 2017

I succeeded to build Firefox 57 with this patch without errors.

I made PR for Firefox 57, without this patch.
#31661

@7c6f434c 7c6f434c merged commit ea1724b into NixOS:master Nov 14, 2017
@7c6f434c
Copy link
Member

Double merge and a hope for the best.

@roastiek
Copy link
Contributor Author

Nice.

I will prepare cleaner version of the patch for firefox. Instead of overriding system dir, it will extend search path for native mess. hosts as MOZ_PLUGIN_PATH for plugins. It should have higher chance to get merged into the upstream. And if not, I have working proof of concept patching omni.ja in the firefox wrapper only.

@chvp chvp mentioned this pull request Jul 4, 2020
Merged
10 tasks
@Zahrun Zahrun mentioned this pull request Apr 6, 2023
Closed
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@7c6f434c 7c6f434c 7c6f434c approved these changes

Assignees
No one assigned
Labels
10.rebuild-darwin: 0 10.rebuild-linux: 11-100
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@roastiek @eqyiel @7c6f434c @taku0 @GrahamcOfBorg