Bug 315855

Summary: Add Client::safeSetGeometry() inc. adjustedSize() and make it the write for the property
Product: [Plasma] kwin Reporter: Fabian <0inkane>
Component: scriptingAssignee: KWin default assignee <kwin-bugs-null>
Status: RESOLVED UNMAINTAINED    
Severity: wishlist CC: xaver.hugl
Priority: NOR    
Version First Reported In: 4.10.0   
Target Milestone: ---   
Platform: Chakra   
OS: Linux   
Latest Commit: Version Fixed/Implemented In:
Sentry Crash Report:
Attachments: Plasma crash report
Dolphin crash report

Description Fabian 2013-02-27 16:29:35 UTC
Executing the following script in the interactive Kwin console (when an instance of dolphin is open) does crash not only but also plasma and KWin becomes unresponsive. I can provide you the backtraces of dolphin and plasma, but I think the problems lies in KWin.
Maybe related to this: I noted that I can set the size of windows such that their width is smaller than minwidth, which crashes at least Dolphin.
var clients = workspace.clientList(); 
clients.forEach(function(c){
  if (c.caption.indexOf("Dolphin" !== -1 )) {
    print("hi");
    c.geometry = (1,1,1,1);
  };
});

Reproducible: Always

Steps to Reproduce:
1. Launch the interactive scripting console with qdbus org.kde.plasma-desktop /MainApplication showInteractiveKWinConsole& 
2. put the script above in it
3. start Dolphin
4. execute the script
Actual Results:  
Plasma and Dolphin crash, KWin becomes unresponsive.

Expected Results:  
Don't know, an error dialog?
Comment 1 Martin Flöser 2013-02-27 16:36:40 UTC
what do you mean by "KWin becomes unresponsive"?
Comment 2 Fabian 2013-02-27 16:41:40 UTC
Actually, I can't reproduce that part anymore. The mouse cursor changed to the one  seen when e.g. in exposé mode, and windows couldn't be moved anymore. After doing a kwin --replace from tty1, this was fixed. But as I said, this doesn't happen anymore after the crashes.
Comment 3 Martin Flöser 2013-02-27 16:50:19 UTC
then I would like to see the crashes for Plasma and Dolphin
Comment 4 Fabian 2013-02-27 16:54:39 UTC
Created attachment 77626 [details]
Plasma crash report
Comment 5 Fabian 2013-02-27 16:55:02 UTC
Created attachment 77627 [details]
Dolphin crash report
Comment 6 Thomas Lübking 2013-02-27 16:58:17 UTC
Hanging kwin could stem from eg. the decoration if the geometry set in scripting is bound to plainresize.
While clients should not crash for any dimension you put on them (kinda client bug) the scipting geometry should somehow go through sizeForClientSize (if it doesn't)
Comment 7 Thomas Lübking 2013-02-27 17:03:58 UTC
"Crash" is abort for Q_ASSERT(src.depth() >= 16);
Script seems to set size of window including deco, so the Window gets invalid.
Function needs to walk through adjustedSize() then
Comment 8 Martin Flöser 2013-02-27 19:50:56 UTC
it goes through Client::setGeometry() - given the code above I think that it does not create a QRect of 1,1x1,1 but an invalid rect. That's a case which could be easily caught in the method. For more I'm unsure whether that is worth the effort.
Comment 9 Thomas Lübking 2013-02-27 20:01:10 UTC
The sanity is currently maintained through adjustedSize() and i'm not sure we can just move that function down to setGeometry()
(at best it would double some cpu cycles)

Question is about the scope of scripts - clients would not be granted such invalid geometries through their configure requests (neither breaking geometry restrictions etc.)
Comment 10 Martin Flöser 2013-02-28 07:55:32 UTC
(In reply to comment #9)
> Question is about the scope of scripts - clients would not be granted such
> invalid geometries through their configure requests (neither breaking
> geometry restrictions etc.)
I want to have as little additional code in KWin core for scripting as possible. In case of this property I see an issue if we change it: setting the value results in a different value as expected. This could destroy quite some calculations (e.g. tiling)
Comment 11 Fabian 2013-02-28 09:15:27 UTC
I can understand this reasoning and I for one won't mind if checking for this is expected by the authors of scripts. But then maybe a warning should be put into the documentation (I can write this too, if you want)?
Short note about tiling: If the window crashes because the rect size is invalid (too small), it destroys the calculation too ;-). Though that needs (on my laptop) almost 20 windows.
Comment 12 Thomas Lübking 2013-02-28 12:31:23 UTC
(In reply to comment #10)
> (In reply to comment #9)
> > Question is about the scope of scripts - clients would not be granted such
> > invalid geometries through their configure requests (neither breaking
> > geometry restrictions etc.)
> I want to have as little additional code in KWin core for scripting as
> possible. In case of this property I see an issue if we change it: setting
> the value results in a different value as expected. This could destroy quite
> some calculations (e.g. tiling)

(OT)
Imho tiling should not override (but consider) geometry restrictions.
That's a rule job and can break when the client tries to reconfigure itself in return.

@Fabian
If you intend to document this, please also refer to minSize(), maxSize() and basicUnit() which restrict the clients geometry (NOTICE: not the decorated clients geometry!)
Comment 13 Zamundaaa 2023-05-07 10:55:56 UTC
Plasma 4 is no longer supported. If this is still an issue on Plasma 5, please reopen