Bug 413044 - Minimize All Script fails to restore all windows
Summary: Minimize All Script fails to restore all windows
Status: RESOLVED FIXED
Alias: None
Product: kwin
Classification: Plasma
Component: scripts (show other bugs)
Version: 5.17.0
Platform: Neon Linux
: NOR normal
Target Milestone: ---
Assignee: Vlad Zahorodnii
URL: https://phabricator.kde.org/D24944
Keywords: regression
: 395473 413138 (view as bug list)
Depends on:
Blocks:
 
Reported: 2019-10-16 16:59 UTC by Matías Morales
Modified: 2020-02-26 16:19 UTC (History)
4 users (show)

See Also:
Latest Commit:
Version Fixed In: 5.17.2
vlad.zahorodnii: ReviewRequest+


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Matías Morales 2019-10-16 16:59:04 UTC
SUMMARY


STEPS TO REPRODUCE
1. Enable MINIMIZE ALL Kwin Script
2. Put a shortcut to the Script (Meta+D for example or CTRL+ALT+D)
3. Type The Shortchut.

OBSERVED RESULT

I have a shortcut to the kwin script to minimize all windows. Meta+D before Plasma 5.17 works flawlessly minimizing all windows, and restore them.

In 5.17 Meta+D only minimize all windows, but it can't restore windows to maximized.

EXPECTED RESULT

Kwin Script MINIMIZE ALL windows, and restore ALL WINDOWS

SOFTWARE/OS VERSIONS
Windows: 
macOS: 
Linux/KDE Plasma: 
(available in About System)
KDE Plasma Version: 5.17
KDE Frameworks Version: 
Qt Version: 

ADDITIONAL INFORMATION
Comment 1 Nate Graham 2019-10-16 22:18:41 UTC
Can reproduce by doing the following:
- Trigger Minimize All
- Open something on the desktop
- Close that thing
- Trigger Minimize All again

Nothing happens.

https://cgit.kde.org/kwin.git/commit/?id=fec0ab818d2460d2b2dd48e698eb2d48a2f1f0ab looks like a possible culprit.
Comment 2 Vlad Zahorodnii 2019-10-17 07:19:52 UTC
@Nate No, that change is fine. The problem is that the script sets a custom property on each minimized client and it seems like QScriptEngine discards that property afterwards. My guess is that QScriptEngine keeps creating new wrapper objects even though we ask to wrap X11Client objects with QScriptEngine::PreferExistingWrapperObject option.

I guess we could work around this problem by checking whether a given client has been minimized by the script as follows

    relevantClients[i].minimizedByScript === true;

However, it will replace one problem with another. We would still have to fix bug 356161 somehow.
Comment 3 Vlad Zahorodnii 2019-10-17 07:20:56 UTC
Edit: relevantClients[i].minimizedByScript !== false;
Comment 4 Vlad Zahorodnii 2019-10-25 11:05:02 UTC
Argh, looks like I introduced this regression

❯ git bisect log
git bisect start
# good: [139edc7a2e8254051e6cdb8e4b74068c16e4e309] SVN_SILENT made messages (.desktop file) - always resolve ours
git bisect good 139edc7a2e8254051e6cdb8e4b74068c16e4e309
# bad: [54ca0dfe12dba24d4fcd140d44ad3c763352d019] Merge branch 'Plasma/5.17'
git bisect bad 54ca0dfe12dba24d4fcd140d44ad3c763352d019
# good: [780cf9812e9225d22240fc5bccb41073f92e8e52] Merge branch 'Plasma/5.12' into Plasma/5.16
git bisect good 780cf9812e9225d22240fc5bccb41073f92e8e52
# bad: [b99d7923e1ed8d065e5516898fefeff76ddfdf23] Delete a leftover after old scripting api
git bisect bad b99d7923e1ed8d065e5516898fefeff76ddfdf23
# bad: [6cd9b2ba256a5e6c63100bfbd8ef9903976eaccd] Install KPackages with kpackage_install_package
git bisect bad 6cd9b2ba256a5e6c63100bfbd8ef9903976eaccd
# bad: [6cd9b2ba256a5e6c63100bfbd8ef9903976eaccd] Install KPackages with kpackage_install_package
git bisect bad 6cd9b2ba256a5e6c63100bfbd8ef9903976eaccd
# good: [60ef4516436308b37392599d97916f21c06b6f93] Merge branch 'Plasma/5.12' into Plasma/5.16
git bisect good 60ef4516436308b37392599d97916f21c06b6f93
# bad: [b9f922b82a06c91d489572ae2d4c971dc01e07d8] Merge branch 'Plasma/5.16'
git bisect bad b9f922b82a06c91d489572ae2d4c971dc01e07d8
# bad: [6df7b2e8634c0a3750ecdb9611cb4cfde6ca2732] Use new logging category directory when ECM>=5.59
git bisect bad 6df7b2e8634c0a3750ecdb9611cb4cfde6ca2732
# bad: [cd7fae95a4c0f1c1524a84cebc81fa44bffa17be] Forward and log exit code of the nested process
git bisect bad cd7fae95a4c0f1c1524a84cebc81fa44bffa17be
# good: [2cff5cb70a9e680f7c14fbd60b00e31143373399] [tabbox] Properly determine depressed modifiers on X11
git bisect good 2cff5cb70a9e680f7c14fbd60b00e31143373399
# bad: [4bbef8d128ce339b4fa08816780c42c40bdd909e] [scripting] Emit clientAdded for Wayland clients
git bisect bad 4bbef8d128ce339b4fa08816780c42c40bdd909e
# bad: [689d89ae4ead722dcf16fc57e9f86a7772724aaa] [scripting] Port the remaining bits of API to AbstractClient
git bisect bad 689d89ae4ead722dcf16fc57e9f86a7772724aaa
# good: [1516595742187116a0d6711222518855778cf88a] [kcmkwin/touchscreen+screenedges] Call KRunner KRunner
git bisect good 1516595742187116a0d6711222518855778cf88a
# first bad commit: [689d89ae4ead722dcf16fc57e9f86a7772724aaa] [scripting] Port the remaining bits of API to AbstractClient
Comment 5 Vlad Zahorodnii 2019-10-25 14:51:22 UTC
Git commit a738ecce85c14b7cc4e7d1f3c1c01c3db1c9c0e7 by Vlad Zahorodnii.
Committed on 25/10/2019 at 14:39.
Pushed by vladz into branch 'Plasma/5.17'.

[scripting] Provide conversion functions for AbstractClient

Summary:
If no conversion functions are provided for a QObject-subclass, then
QScriptEngine will use QScriptEngine::newQObject() method without any
special options to convert an instance of that QObject-subclass to a
QScriptValue. However, it's very important that every client object is
wrapped with PreferExistingWrapperObject option. We need that option
because a script may set a property on a client object and that property
must remain until it's deleted by the script.
FIXED-IN: 5.17.2

Test Plan: New test.

Reviewers: #kwin, davidedmundson

Reviewed By: #kwin, davidedmundson

Subscribers: davidedmundson, kwin

Tags: #kwin

Differential Revision: https://phabricator.kde.org/D24944

M  +1    -0    autotests/integration/scripting/CMakeLists.txt
A  +170  -0    autotests/integration/scripting/minimizeall_test.cpp     [License: GPL (v2)]
M  +15   -0    scripting/meta.cpp
M  +8    -0    scripting/meta.h
M  +12   -4    scripts/CMakeLists.txt

https://commits.kde.org/kwin/a738ecce85c14b7cc4e7d1f3c1c01c3db1c9c0e7
Comment 6 Patrick Silva 2020-01-29 01:40:46 UTC
*** Bug 413138 has been marked as a duplicate of this bug. ***
Comment 7 Nate Graham 2020-02-26 16:19:43 UTC
*** Bug 395473 has been marked as a duplicate of this bug. ***