Bug 330102

Summary:	KWin::Deleted *deleted in windowClosed is not properly converted to javascript
Product:	[Plasma] kwin	Reporter:	Fabian Homborg <FHomborg>
Component:	scripting	Assignee:	KWin default assignee <kwin-bugs-null>
Status:	RESOLVED FIXED
Severity:	minor
Priority:	NOR
Version:	4.11.5
Target Milestone:	---
Platform:	unspecified
OS:	Linux
Latest Commit:		Version Fixed In:
Sentry Crash Report:

Description Fabian Homborg 2014-01-17 20:44:11 UTC

It seems that the *deleted attribute of the windowClosed signal isn't converted to javascript types/objects, which results in an ugly warning in kwin output and not being usable by javascript (though the documentation doesn't mention what it is, and I don't think I need it).

The signal itself and handling it works.

Reproducible: Always

Steps to Reproduce:
Connect to windowClosed in a script.
Actual Results:  
QScriptEngine: Unable to handle unregistered datatype 'KWin::Deleted*' when invoking handler of signal KWin::Client::windowClosed(KWin::Toplevel*,KWin::Deleted*)

in console output or ~/.xsession-errors and no usable attribute in javascript.

Expected Results:  
Nothing in console and a properly usable attribute in javascript

Comment 1 Martin Flöser 2014-01-17 21:04:42 UTC

is that a KWin script or a KWin effect? Both should not know anything about 
Deleted. That's internal information which should not be exposed. So might 
need a reconnection to get rid of the signal argument.

Care to share your example code for testing?

Comment 2 Thomas Lübking 2014-01-17 21:24:09 UTC

The signal is used by the scene (to rebind the EffectWindow)

Either there's a way to prevent signals from script export or we could turn it into

/** deleted is only for internal use, hands off! */
void windowClosed(KWin::Toplevel *t, void *deleted);

To prevent the script warning.

@Fabian
can you bind workspace.clientRemoved(c)?

Comment 3 Fabian Homborg 2014-01-18 00:15:14 UTC

My real code is at https://github.com/faho/kwin-tiling (in tilelist.js), but this example code triggers the warning as well:

workspace.clientAdded.connect(function(cl) {
	cl.windowClosed.connect(function(client, deleted) {
		print("With both arguments");
	});
	cl.windowClosed.connect(function(client) {
		print("With one argument");
	});
	cl.windowClosed.connect(function() {
		print("Without arguments");
	});
});

However, the important thing is that windowClosed works and executes my function, and does it before FFM selects a new window. That's the issue why I no longer use clientRemoved - I need to resize the windows before FFM selects something, or find the window under the mouse (though the latter might race with focus stealing etc).

Comment 4 Martin Flöser 2014-01-18 07:38:00 UTC

> However, the important thing is that windowClosed works and executes my
> function, and does it before FFM selects a new window. That's the issue why
> I no longer use clientRemoved - I need to resize the windows before FFM
> selects something, or find the window under the mouse (though the latter
> might race with focus stealing etc).

Relying on the internal code execution order in KWin - that sounds dangerous 
;-) Though it should be fine in that case. I think the easiest way is that we 
export Deleted* to the scripting engine. But I have to think more about the 
possible negative impact of providing Deleted to scripts.

Comment 5 Thomas Lübking 2014-01-18 14:59:10 UTC

It would be cool if there was a general way to make QtScript treat a pointer to an undefined type as long uint (see tabgroup case)

Comment 6 Andrew Crouthamel 2018-11-11 04:27:35 UTC

Dear Bug Submitter,

This bug has been stagnant for a long time. Could you help us out and re-test if the bug is valid in the latest version? I am setting the status to NEEDSINFO pending your response, please change the Status back to REPORTED when you respond.

Thank you for helping us make KDE software even better for everyone!

Comment 7 Andrew Crouthamel 2018-11-21 04:21:05 UTC

Dear Bug Submitter,

This is a reminder that this bug has been stagnant for a long time. Could you help us out and re-test if the bug is valid in the latest version? This bug will be moved back to REPORTED Status for manual review later, which may take a while. If you are able to, please lend us a hand.

Thank you for helping us make KDE software even better for everyone!

Comment 8 Fabian Homborg 2018-11-22 16:45:55 UTC

This actually seems to be solved? At least I'm not getting an error anymore.