Bug 439529 - Assert _currentTerminalDisplay in Screen::setTextSelectionRendition()
Summary: Assert _currentTerminalDisplay in Screen::setTextSelectionRendition()
Status: RESOLVED FIXED
Alias: None
Product: konsole
Classification: Applications
Component: general (show other bugs)
Version: master
Platform: Other Linux
: NOR crash
Target Milestone: ---
Assignee: Konsole Developer
URL:
Keywords:
: 439626 (view as bug list)
Depends on:
Blocks:
 
Reported: 2021-07-05 20:56 UTC by ninjalj
Modified: 2021-07-08 20:43 UTC (History)
3 users (show)

See Also:
Latest Commit:
Version Fixed In: 21.08
Sentry Crash Report:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
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