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

PCManFM-Qt doesn't minimize when pressing the "show desktop" button #1411

Closed
iamthesenate1 opened this issue Jul 1, 2021 · 27 comments
Closed

Comments

@iamthesenate1
Copy link
Contributor

iamthesenate1 commented Jul 1, 2021

Expected Behavior

It should minimize too, so that the desktop is shown.

Current Behavior

It doesn't.

Steps to Reproduce (for bugs)
  1. Have PCManFM-Qt open.
  2. Press the "show desktop" button.
System Information
  • Distribution & Version: Arch Linux, LXQt-git from the archlinuxcn repo.
  • Kernel: 5.13
  • Qt Version: 5.15.2
  • libfm-qt Version: git
  • libqtxdg Version: git
  • lxqt-build-tools Version: git
  • Package version: git
@stefonarch
Copy link
Member

stefonarch commented Jul 1, 2021

Not reproducible, using git version on 2 machines (arch|debian).
Probably due some compiling errors?

EDIT: installed from AUR?

@iamthesenate1
Copy link
Contributor Author

No, installed from the archlinuxcn repo

@stefonarch
Copy link
Member

So this should be reported to the maintainers https://wiki.archlinux.org/title/Unofficial_user_repositories#archlinuxcn.
Make sure that you have no mixed installation, so everything should be from this repo.

@iamthesenate1
Copy link
Contributor Author

iamthesenate1 commented Jul 1, 2021

This might not be a packaging problem or pcmanfm-qt's, but because of my WM (KwinFT), because I have now built all lxqt packages from source and the problem persists.

@iamthesenate1
Copy link
Contributor Author

Can confirm. Openbox works fine.

@tsujan
Copy link
Member

tsujan commented Jul 1, 2021

@iamthesenate1
Minimizing windows and showing Desktop is the job of WM; it isn't related to pcmanfm-qt or other apps.

Some WMs may see that LXQt's Desktop is drawn by pcmanfm-qt and treat all its windows as "desktop". KWin has that bug. There may be a workaround for KWin because it has so many settings but I'm not sure.

@iamthesenate1
Copy link
Contributor Author

@iamthesenate1
Minimizing windows and showing Desktop is the job of WM; it isn't related to pcmanfm-qt or other apps.

Some WMs may see that LXQt's Desktop is drawn by pcmanfm-qt and treat all its windows as "desktop". KWin has that bug. There may be a workaround for KWin because it has so many settings but I'm not sure.

I understand now. Thanks for the information.

@antis81
Copy link

antis81 commented Sep 5, 2021

@tsujan It's a bug 🐞 in pcmanfm-qt (and it can reproduced)!

Basically openbox cannot handle a virtual desktop well. So pcmanfm-qt --desktop steps in and provides the functionality of a desktop manager. This isn't a problem with KWin(FT). However systemsettings package depends on plasma-workspace - a second desktop manager 🎉. Now we have 2 desktop managers and the responsibility is unclear! 💥

Currently the only way to resolve this is to uninstall systemsettings and configure everything(!) KDE related manually via config files!

EDIT: See KCM discussion

EDIT: Uninstalling plasma-workspace does not have an impact. This can only be solved in pcmanfm-qt itself.

@tsujan
Copy link
Member

tsujan commented Sep 5, 2021

It's a bug lady_beetle in pcmanfm-qt

No, it isn't a bug in pcmanfm-qt. Its KWin's job to recognize what window has _NET_WM_WINDOW_TYPE_DESKTOP in its X11 window property NET_WM_WINDOW_TYPE and what doesn't. Openbox distinguishes between them. We set that property for the desktop by adding the attribute Qt::WA_X11NetWmWindowTypeDesktop.

See KCM discussion

Not relevant to this.

@antis81
Copy link

antis81 commented Sep 5, 2021

@tsujan Why does it work when starting pcmanfm-qt without the --desktop option? (see /etc/xdg/autostart/lxqt-desktop.desktop)

@tsujan
Copy link
Member

tsujan commented Sep 5, 2021

Why does it work when starting pcmanfm-qt without the --desktop option?

Because KWin doesn't mistake it for desktop anymore. Not distinguishing between windows that have the above-mentioned X11 property and those that don't, KWin sees that property in the desktop and then treats all pcmanfm-qt windows as if they were all "desktops". Openbox doesn't make that mistake.

@antis81
Copy link

antis81 commented Sep 5, 2021

@tsujan Okay since I haven't found anything in that direction in KDE's bug tracker, I will file a bug there (10 year old account surprisingly still works - wee 🎉!)

@tsujan
Copy link
Member

tsujan commented Sep 5, 2021

That's a good idea.

@antis81
Copy link

antis81 commented Sep 5, 2021

For reference: https://bugs.kde.org/show_bug.cgi?id=442044

@antis81
Copy link

antis81 commented Sep 6, 2021

@tsujan Finally I understand the full problem! We can actually solve this by using smart-pointers our own pointer list instead of parenting the QMainWindow to the (single) pcmanfm QApplication instance (works also on openbox)!

Would you provide the dbus call please so I can debug it (e.g. map a shortcut)?

@stefonarch
Copy link
Member

(works also on openbox)!

make sure it works also with xfwm4

@antis81
Copy link

antis81 commented Sep 6, 2021

@stefonarch This is going to be WM agnostic -> works with any WM! 😺

@tsujan
Copy link
Member

tsujan commented Sep 6, 2021

@antis81
I don't understand what you're talking about. pcmanfm-qt is a single-instance app. We don't change that just because KWin can't handle it in this special case.

@tsujan
Copy link
Member

tsujan commented Sep 6, 2021

Also see lxqt/lxqt-panel#1477 (especially, lxqt/lxqt-panel#1477 (comment)). If it doesn't exist for you, you need a more complete KDE installation.

@antis81
Copy link

antis81 commented Sep 6, 2021

@stefonarch This will also work in xfwm4 (it will work in any WM practically!)

@tsujan Do you understand how Qt's "garbage collection" works (QObject parenting)? (I have written similar architectures before and it can solve quite some problems here.) In the end we both want to stay independent from KDE don't we?

Draft:

// file: libfm-qt/filedialog.cpp
namespace Fm {

QList<QWidget*> floatingWindows;
//TODO:
// - Move floatingWindows to a common place where pcmanfm-qt process has access as well.
// - call "qDeleteAll(Fm::floatingWindows)" (maybe "for (auto w : Fm::floatingWindows) { w->close(); }") before pcmanfm-qt process ends.

constexpr QWidget* parentWindow(QWidget* parent, QWidget* thiz) {
    if (parent->testAttribute(Qt::WA_X11NetWmWindowTypeDesktop)) {
        floatingWindows << thiz;
        return nullptr;
    }

    return parent;
}


FileDialog::FileDialog(QWidget* parent, FilePath path) :
    QDialog(parentWindow(parent, this)),
//
{
     //
}

FileDialog::~FileDialog() {
    floatingWindows.removeAll(this);
    //
}

}

Note: As a QDialog is also a window this should work right away! For floating QWidget's we might wrap that into a QMainWindow() or QDialog().

Are we good now? ❤️ 🙂

@antis81
Copy link

antis81 commented Sep 8, 2021

FYI: Wrote a little test application that cleanly reproduces this for KDE!

@antis81
Copy link

antis81 commented Sep 16, 2021

@tsujan We have actually located the bug -> https://invent.kde.org/plasma/kwin/-/blob/master/src/layers.cpp#L736

The belongsToDesktop function cycles all of an application's "top-level" windows and as soon as one of those has the _NET_WM_WINDOW_TYPE_DESKTOP makes the false assumption, that every window belongs to the desktop. Now for the fixing… ☕ 🙂

(Since this is is a virtual function, KWin takes a different code path in Wayland and it works there! Maybe we can port the logic apply it to the X11 client… just thinking out loud here… 🤔 📢)

@tsujan
Copy link
Member

tsujan commented Sep 16, 2021

Very good. Hopefully, it'll be fixed in the next version.

@antis81
Copy link

antis81 commented Sep 20, 2021

@tsujan
Copy link
Member

tsujan commented Sep 20, 2021

Well done.

@antis81
Copy link

antis81 commented Sep 20, 2021

Not sure if it will work with plasma's "panel" (couldn't find the code). It likely doesn't set one of those flags (LXQt uses Qt::WA_X11NetWmWindowTypeDock for the panel).

@tsujan
Copy link
Member

tsujan commented Sep 20, 2021

They'll review the code.

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

No branches or pull requests

4 participants