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

Make X11Extras conditional on autotype again #7635

Closed
wants to merge 1 commit into from

Conversation

Hello71
Copy link
Contributor

@Hello71 Hello71 commented Mar 24, 2022

After 404fd94 ("Move global shortcut handling into OSUtils
(#5566)"), X11Extras was made mandatory, even though it is only used for
autotype. Make it conditional on autotype again.

Testing strategy

Tested that with autotype disabled and X11Extras not installed, KeePassXC successfully compiles, opens database, and opens application settings. Did not test autotype enabled.

Type of change

  • ✅ Bug fix (non-breaking change that fixes an issue)

After 404fd94 ("Move global shortcut handling into OSUtils
(keepassxreboot#5566)"), X11Extras was made mandatory, even though it is only used for
autotype. Make it conditional on autotype again.
@codecov-commenter
Copy link

codecov-commenter commented Mar 24, 2022

Codecov Report

❗ No coverage uploaded for pull request base (develop@4178e72). Click here to learn what that means.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##             develop    #7635   +/-   ##
==========================================
  Coverage           ?   64.41%           
==========================================
  Files              ?      339           
  Lines              ?    43344           
  Branches           ?        0           
==========================================
  Hits               ?    27920           
  Misses             ?    15424           
  Partials           ?        0           
Impacted Files Coverage Δ
src/gui/osutils/nixutils/NixUtils.cpp 42.86% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4178e72...d5c87ba. Read the comment docs.

@droidmonkey
Copy link
Member

We are planning to use global shortcuts for things other than Auto-Type.

@Hello71
Copy link
Contributor Author

Hello71 commented Mar 24, 2022

Is it possible to add a XC_X11 then? The keepassxc -> qtx11extras -> X qtgui dependency chain is quite unfortunate for Wayland users of source-based distros.

@droidmonkey
Copy link
Member

Sure I can get behind that, as long as you successfully kill off all x11 dependency. Just removing qt5x11 is not enough.

@Hello71
Copy link
Contributor Author

Hello71 commented Mar 24, 2022

Sure I can get behind that, as long as you successfully kill off all x11 dependency. Just removing qt5x11 is not enough.

I'm not sure what you mean. I am currently successfully running keepass 2.7.0 plus this PR with qtgui compiled without X support, as defined by Gentoo.

@droidmonkey
Copy link
Member

Nevermind I was confused. What is even provided by qt5x11extras? The documentation is blank from qt.

@jirutka
Copy link

jirutka commented Apr 7, 2022

I’m with @Hello71, qt11extras is a pretty huge dependency and it’s unnecessary for Wayland users. I’m thinking about using this patch in Alpine package to avoid this dependency chain. However, since it’s part of the application, not just autotype plugin, I have to build KeePassXC twice – with and without autotype – and provide two packages – one for X11 users and the other one for Wayland users. That’s quite suboptimal.

@Hello71
Copy link
Contributor Author

Hello71 commented Apr 7, 2022

What is even provided by qt5x11extras? The documentation is blank from qt.

Well, it provides at least the QX11Info which is disabled by this PR. That is necessary to bind global hotkeys using X11 API.

However, since it’s part of the application, not just autotype plugin, I have to build KeePassXC twice – with and without autotype – and provide two packages – one for X11 users and the other one for Wayland users. That’s quite suboptimal.

I agree, but I think this was true even before 2.7.0. Unfortunately, I don't think there's an easy way to avoid this. I think the "correct" solution is to replace X11 application-internal binding with global DE binding (via desktop file?). That is the only way forward for Wayland. However, Auto-Type will still not work. Probably the right move there is to open a separate project handling simulated keyboard input across X11 and Wayland, and then allow that project to be compiled with X or Wayland support or both.

@michaelk83
Copy link

I think the "correct" solution is to replace X11 application-internal binding with global DE binding (via desktop file?). That is the only way forward for Wayland.

There was some talk (and a draft PR) of a global keyboard shortcut portal, but it's not ready yet: #2281 (comment) (follow the links from there).

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

Successfully merging this pull request may close these issues.

5 participants