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

Revert "webkitgtk: 2.34.6 → 2.36.0" #170070

Closed
wants to merge 1 commit into from

Conversation

edolstra
Copy link
Member

Description of changes

This reverts commit e2b791c, since it broke reports in gnucash. That is, all reports simply showed a blank screen.

CC @jtojnar

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 22.05 Release Notes (or backporting 21.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

This reverts commit e2b791c, since it
broke reports in gnucash.
@jtojnar
Copy link
Member

jtojnar commented Apr 24, 2022

We cannot revert since the update is security critical (fixes some CVEs). As a workaround, you can set WEBKIT_DISABLE_COMPOSITING_MODE=1 environment variable to disable hardware accelerated compositing on the platforms that are buggy.

@edolstra
Copy link
Member Author

We shouldn't break stuff on the stable branch, and I doubt gnucash's usage of webkitgtk as a renderer is very security-critical.

The hardware is nvidia btw, not exactly an obscure platform that we can just break.

@jtojnar
Copy link
Member

jtojnar commented Apr 25, 2022

We shouldn't break stuff on the stable branch.

Agreed, but we cannot just leave webkitgtk insecure since it is used in untrusted contexts (e.g. web browsers).

  1. So we would either need to set

environment.variables.WEBKIT_DISABLE_COMPOSITING_MODE = "1";

in some NixOS module,

  1. Pass the variable to all apps using webkitgtk (see e.g. PR that disabled the HW-accelerated compositing in Evolution e-mail client.

  2. Re-introduce the insecure webkitgtk and have packages that work with HTML that breaks the compositing use that.

@hmenke
Copy link
Member

hmenke commented Apr 26, 2022

Yet another alternative to environment variables is patching WebKit to disable hardware acceleration as proposed in #169058

@jtojnar
Copy link
Member

jtojnar commented Apr 26, 2022

Right, forgot about that. Though that will likely make element-heavy sites like YouTube unusable in WebkitGTK-based browsers.

@jonringer
Copy link
Contributor

jonringer commented Apr 26, 2022

if gnucash is the one of the exceptions, we should probably avoid doing system wide changes and just wrap the affected applications with WEBKIT_DISABLE_COMPOSITING_MODE=1 before running

@jtojnar
Copy link
Member

jtojnar commented Apr 29, 2022

The point release mentions some rendering fixes #170905. Could you try that?

@jtojnar jtojnar mentioned this pull request May 2, 2022
@StephenWithPH StephenWithPH mentioned this pull request May 11, 2022
13 tasks
@edolstra
Copy link
Member Author

Unfortunately webkitgtk 2.36.1 does not fix the problem. However, the WEBKIT_DISABLE_COMPOSITING_MODE=1 workaround does work, so I'll use that.

@edolstra edolstra closed this May 14, 2022
@edolstra edolstra deleted the fix-gnucash branch May 14, 2022 06:58
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.

4 participants