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

🙈️ Remove webkitgtk24x #75040

Merged
merged 9 commits into from Dec 7, 2019

Conversation

worldofpeace
Copy link
Contributor

Motivation for this change

I hope it goes without saying, but please 😂️

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 nix-review --run "nix-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)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@worldofpeace worldofpeace changed the title Remove webkitgtk24x 🙈️ 🙈️ Remove webkitgtk24x Dec 5, 2019
@worldofpeace worldofpeace added this to To do in Picking up garbage via automation Dec 5, 2019
@worldofpeace worldofpeace moved this from To do to In progress in Picking up garbage Dec 5, 2019
@worldofpeace
Copy link
Contributor Author

#18312 basically did this in an elegant way, but we can for sure remove this now.

@worldofpeace
Copy link
Contributor Author

#18312 basically did this in an elegant way, but we can for sure remove this now.

It uses insecure webkitgtk24x.
You know cannot enable optional withWebKit when
withGtk2 is enabled.
@jtojnar
Copy link
Contributor

jtojnar commented Dec 5, 2019

@worldofpeace
Copy link
Contributor Author

@jtojnar
Copy link
Contributor

jtojnar commented Dec 5, 2019

See mmex in all-packages

@worldofpeace
Copy link
Contributor Author

@jtojnar Running unpackPhase on emacs25 shows

  WEBKIT_REQUIRED=1.4.0
  WEBKIT_MODULES="webkitgtk-3.0 >= $WEBKIT_REQUIRED"

So I think the source in nixpkgs predates this commit.

@worldofpeace
Copy link
Contributor Author

See mmex in all-packages

Did 3e1ee5b.
This reminds me wxGTK needs to default to gtk3 #73145, but it's pretty tedious TBH.

uzbl/uzbl#408

Frightening version of webkitgtk used here.
😀. This for sure has been superseded in the present.
Nothing uses this in nixpkgs also.
It goes without saying that we should remove this 😅️.
@jtojnar
Copy link
Contributor

jtojnar commented Dec 5, 2019

https://git.savannah.gnu.org/cgit/emacs.git/commit/src/xwidget.c?id=a36ed9b5e95afea5716256bac24d883263aefbaf seems to build:

diff --git a/pkgs/applications/editors/emacs/25.nix b/pkgs/applications/editors/emacs/25.nix
index f3989be52c0..15e9fbdd726 100644
--- a/pkgs/applications/editors/emacs/25.nix
+++ b/pkgs/applications/editors/emacs/25.nix
@@ -48,7 +48,16 @@ stdenv.mkDerivation rec {
     })
   ] ++ [
     # Backport patch so we can use webkitgtk with xwidgets.
-    ./0001-xwidget-Use-WebKit2-API.patch
+    (fetchurl {
+      name = "0001-Omit-unnecessary-includes-from-xwidget-c.patch";
+      url = "https://git.savannah.gnu.org/cgit/emacs.git/patch?id=a36ed9b5e95afea5716256bac24d883263aefbaf";
+      sha256 = "0a4ax1kjcn6g4c59a0wikplx47r6r3lh5igc9vrlqkpk5lw30j0j";
+    })
+    (fetchurl {
+      name = "0002-xwidget-Use-WebKit2-API.patch";
+      url = "https://git.savannah.gnu.org/cgit/emacs.git/patch?id=d781662873f228b110a128f7a2b6583a4d5e0a3a";
+      sha256 = "1m2fnblsk4dg3scxiawz6vsa83bvyg9bxis9mhrjd5f7cdmcnn4z";
+    })
   ];
 
   nativeBuildInputs = [ pkgconfig autoconf automake texinfo ]

@worldofpeace
Copy link
Contributor Author

@jtojnar Fetched those patches. I had to do it from the GitHub mirror because of the ongoing DOS (or maybe I'm blacklisted).

@worldofpeace worldofpeace merged commit 0c8dadd into NixOS:master Dec 7, 2019
Picking up garbage automation moved this from In progress to Done Dec 7, 2019
@worldofpeace worldofpeace deleted the remove-terror-webkitgtk branch December 7, 2019 03:02
@orivej
Copy link
Contributor

orivej commented Dec 8, 2019

Claws Mail will build with a supported WebKit once it switches to GTK 3, which seems to progress well:
https://git.claws-mail.org/?p=claws.git;a=shortlog;h=refs/heads/gtk3
https://git.claws-mail.org/?p=claws.git;a=history;f=src/plugins/fancy;hb=refs/heads/gtk3
Meanwhile Claws Mail 3.17.4 has added a new litehtml-based HTML renderer plugin. I suppose I'll try to package it, since lack of HTML support noticebly reduces its usability. Alternatively it would make sense to provide a gtk3 branch snapshot of Claws Mail.
EDIT: Note that Claws Mail uses webkitgtk with javascript disabled, and therefore does not put its users at risk of known security issues.

@orivej
Copy link
Contributor

orivej commented Dec 9, 2019

I have tried the litehtml plugin — it is as simple as adding gumbo to claws-mail build inputs (and enabling it in Configuration > Plugins) — but it's rendering is very bad, with huge spacing between words and lines, and the text can not be selected and copied.

@worldofpeace
Copy link
Contributor Author

worldofpeace commented Dec 9, 2019

Claws Mail will build with a supported WebKit once it switches to GTK 3, which seems to progress well:
https://git.claws-mail.org/?p=claws.git;a=shortlog;h=refs/heads/gtk3
https://git.claws-mail.org/?p=claws.git;a=history;f=src/plugins/fancy;hb=refs/heads/gtk3
Meanwhile Claws Mail 3.17.4 has added a new litehtml-based HTML renderer plugin. I suppose I'll try to package it, since lack of HTML support noticebly reduces its usability. Alternatively it would make sense to provide a gtk3 branch snapshot of Claws Mail.

It seems the only mostly working solution would be a gtk3 variant package. Though it's not good to distribute non-released/non-production code (some people even request that you don't), it's an option.

@jtojnar
Copy link
Contributor

jtojnar commented Dec 9, 2019

EDIT: Note that Claws Mail uses webkitgtk with javascript disabled, and therefore does not put its users at risk of known security issues.

While disabling JavaScript will reduce the attack surface greatly, I doubt JavaScriptCore is the only vulnerable component.

orivej-nixos pushed a commit that referenced this pull request Mar 13, 2020
This branch currently seems an almost adequate replacement for gtk2 claws-mail,
except that clicking links in the web view opens them in the email window even
when "open links with external browser" is enabled.

Related: #75040
dtzWill pushed a commit to dtzWill/nixpkgs that referenced this pull request Mar 14, 2020
This branch currently seems an almost adequate replacement for gtk2 claws-mail,
except that clicking links in the web view opens them in the email window even
when "open links with external browser" is enabled.

Related: NixOS#75040
(cherry picked from commit b0d9764)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants