Bug 439529

Summary: Assert _currentTerminalDisplay in Screen::setTextSelectionRendition()
Product: [Applications] konsole Reporter: ninjalj
Component: generalAssignee: Konsole Developer <konsole-devel>
Status: RESOLVED FIXED    
Severity: crash CC: a.samirh78, bharadwaj.raju777, nate
Priority: NOR    
Version: master   
Target Milestone: ---   
Platform: Other   
OS: Linux   
Latest Commit: https://invent.kde.org/utilities/konsole/commit/877a5128cb7cb3af51b9f9b360355bbb2a58269e Version Fixed In: 21.08
Sentry Crash Report:

Description ninjalj 2021-07-05 20:56:08 UTC 
SUMMARY

Q_ASSERT(_currentTerminalDisplay) on Screen::setTextSelectionRendition() triggers on a newly opened konsole after Edit -> Select All.

STEPS TO REPRODUCE
1. Launch a new Konsole.
2. Do NOT interact with the TerminalDisplay widget.
3. On the menubar, select "Edit" → "Select All".

OBSERVED RESULT

ASSERT: "_currentTerminalDisplay" in file /home/lj/src/term/konsole/src/Screen.cpp, line 658
Abortado


Screen::_currentTerminalDisplay is not set until one of the following methods is called:

TerminalDisplay::mousePressEvent()
TerminalDisplay::wheelEvent()
TerminalDisplay::keyPressEvent()


Maybe Screen::_currentTerminalDisplay should be set on Session::addView() ?
Comment 1 Ahmad Samir 2021-07-06 22:29:35 UTC 
It took a bit of trial and error to get it to crash.
- open a konsole window
- don't use the keyboard (using e.g. Alt+E to open the Edit menu means that setCurrentTerminalDisplay() is called from the keyPressEvent())
- don't move the mouse over the terminal area
- move the mouse from the titlebar to the edit menu and "select all"
then it crashes
Comment 2 Ahmad Samir 2021-07-06 22:31:14 UTC 
I've created https://invent.kde.org/utilities/konsole/-/merge_requests/433

I don't see why setTerminalDisplay was being called in various places in the code, there is a reason, I just don't see it...
Comment 3 ninjalj 2021-07-07 15:26:53 UTC 
Moving the mouse shouldn't be a problem for reproducing the problem, as long as there are no button presses or wheel movement.
Comment 4 Bug Janitor Service 2021-07-07 15:32:17 UTC 
A possibly relevant merge request was started @ https://invent.kde.org/utilities/konsole/-/merge_requests/434
Comment 5 ninjalj 2021-07-07 15:35:25 UTC 
As for the reason setTerminalDisplay was being called in various places in the code, Vt102Emulation::sendKeyEvent() uses it to know if the session is read-only, and to call TerminalDisplay::scrollScreenWindow().
Comment 6 Ahmad Samir 2021-07-08 06:45:05 UTC 
*** Bug 439626 has been marked as a duplicate of this bug. ***
Comment 7 Kurt Hindenburg 2021-07-08 20:42:41 UTC 
Git commit 877a5128cb7cb3af51b9f9b360355bbb2a58269e by Kurt Hindenburg, on behalf of Luis Javier Merino Morán.
Committed on 08/07/2021 at 20:42.
Pushed by hindenburg into branch 'master'.

Fix assert _currentTerminalDisplay in Screen::setTextSelectionRendition

_currentTerminalDisplay was not set until some interaction was made with
the TerminalDisplay widget.  Set it as soon as Session->addView() is
called.

M  +6    -0    src/Emulation.cpp
M  +6    -0    src/Emulation.h
M  +2    -0    src/session/Session.cpp
M  +0    -5    src/terminalDisplay/TerminalDisplay.cpp

https://invent.kde.org/utilities/konsole/commit/877a5128cb7cb3af51b9f9b360355bbb2a58269e