Bug 432077 - Konsole flashes when it's closed by pressing Ctrl-D
Summary: Konsole flashes when it's closed by pressing Ctrl-D
Status: RESOLVED FIXED
Alias: None
Product: konsole
Classification: Applications
Component: general (show other bugs)
Version: master
Platform: Other Linux
: NOR normal
Target Milestone: ---
Assignee: Konsole Developer
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-01-25 08:11 UTC by Vlad Zahorodnii
Modified: 2021-09-03 00:04 UTC (History)
4 users (show)

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


Attachments
video that demonstrates the issue (3.72 MB, video/mp4)
2021-01-25 08:11 UTC, Vlad Zahorodnii
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Vlad Zahorodnii 2021-01-25 08:11:47 UTC
Created attachment 135151 [details]
video that demonstrates the issue

SUMMARY


STEPS TO REPRODUCE
1. Reduce animation speed in system settings
2. Open Konsole
3. Close it by pressing Ctrl-D

OBSERVED RESULT
Konsole flashes.

EXPECTED RESULT
No flashing.

ADDITIONAL INFORMATION
This bug doesn't occur if you close Konsole by clicking the close button in system settings.
Comment 1 Dave Flogeras 2021-08-10 19:27:57 UTC
I also just observed this behaviour since upgrading from 20.12.3 -> 21.04.3.  Downgrading just konsole eliminates the behaviour so I don't think it's related to frameworks etc..

Is there a workaround (other than using the mouse which I'm allergic to :)?  This is quite harsh on the eyes in a dark room.  I'm using Gentoo so happy to test patches.
Comment 2 Bug Janitor Service 2021-08-12 12:25:26 UTC
A possibly relevant merge request was started @ https://invent.kde.org/utilities/konsole/-/merge_requests/456
Comment 3 Ahmad Samir 2021-08-12 12:42:45 UTC
Git commit bbec72250d080ce286a6762fb9beee4b6e7981c9 by Ahmad Samir.
Committed on 12/08/2021 at 12:25.
Pushed by tcanabrava into branch 'master'.

Prevent window "flashing" when closing the last session

There are two scenarios when closing a window:
A) clicking the close button on the title bar (or Ctrl+Shift+Q):
~MainWindow()
~ViewManager()
~TabbedViewContainer()
~TerminalDisplay()
~Session()

B) closing the last session/tab in a window:
SessionController::sessionFinished()
~Session()
~TerminalDisplay()
~TabbedViewContainer()
~MainWindow()
~ViewManager()

the issue with the second case is that the TerminalDisplay is torn down
first, which exposes the TabbedViewContainer widget, the latter has the same
Qt::Window colour as the system colour scheme window background colour, if
you're using a dark terminal colour scheme and a light-coloured system colour
scheme, you could see some "flashing" when you close the last session with
e.g. Ctrl+D.

To fix this, in sessionFinished() check if TabbedViewContainer::count() is
1 (i.e. closing last tab/session), and emit the empty() signal in that case,
which is connected to MainwWindow::close(), then the order of tear down
becomes:
SessionController::sessionFinished()
~Session()
~MainWindow()
~ViewManager()
~TabbedViewContainer()
~TerminalDisplay()
FIXED-IN: 21.12

M  +1    -1    src/MainWindow.cpp
M  +7    -0    src/ViewManager.cpp

https://invent.kde.org/utilities/konsole/commit/bbec72250d080ce286a6762fb9beee4b6e7981c9
Comment 4 Dave Flogeras 2021-08-12 23:22:44 UTC
Thank you very much, it applies cleanly and works here on 21.04.3.
Comment 5 Andreas Sturmlechner 2021-08-13 12:59:57 UTC
What about 20.08 branch?
Comment 6 Ahmad Samir 2021-08-13 13:06:20 UTC
Git commit c8d60923728fb7b16eca613a44ee47e90ab86c72 by Ahmad Samir.
Committed on 13/08/2021 at 13:06.
Pushed by ahmadsamir into branch 'release/21.08'.

Prevent window "flashing" when closing the last session

There are two scenarios when closing a window:
A) clicking the close button on the title bar (or Ctrl+Shift+Q):
~MainWindow()
~ViewManager()
~TabbedViewContainer()
~TerminalDisplay()
~Session()

B) closing the last session/tab in a window:
SessionController::sessionFinished()
~Session()
~TerminalDisplay()
~TabbedViewContainer()
~MainWindow()
~ViewManager()

the issue with the second case is that the TerminalDisplay is torn down
first, which exposes the TabbedViewContainer widget, the latter has the same
Qt::Window colour as the system colour scheme window background colour, if
you're using a dark terminal colour scheme and a light-coloured system colour
scheme, you could see some "flashing" when you close the last session with
e.g. Ctrl+D.

To fix this, in sessionFinished() check if TabbedViewContainer::count() is
1 (i.e. closing last tab/session), and emit the empty() signal in that case,
which is connected to MainwWindow::close(), then the order of tear down
becomes:
SessionController::sessionFinished()
~Session()
~MainWindow()
~ViewManager()
~TabbedViewContainer()
~TerminalDisplay()
FIXED-IN: 21.12
(cherry picked from commit bbec72250d080ce286a6762fb9beee4b6e7981c9)

M  +1    -1    src/MainWindow.cpp
M  +7    -0    src/ViewManager.cpp

https://invent.kde.org/utilities/konsole/commit/c8d60923728fb7b16eca613a44ee47e90ab86c72
Comment 7 Ahmad Samir 2021-08-13 13:08:16 UTC
Git commit 302c16791935cc3cf262aee355afce13d694b00f by Ahmad Samir.
Committed on 13/08/2021 at 13:06.
Pushed by ahmadsamir into branch 'release/21.04'.

Prevent window "flashing" when closing the last session

There are two scenarios when closing a window:
A) clicking the close button on the title bar (or Ctrl+Shift+Q):
~MainWindow()
~ViewManager()
~TabbedViewContainer()
~TerminalDisplay()
~Session()

B) closing the last session/tab in a window:
SessionController::sessionFinished()
~Session()
~TerminalDisplay()
~TabbedViewContainer()
~MainWindow()
~ViewManager()

the issue with the second case is that the TerminalDisplay is torn down
first, which exposes the TabbedViewContainer widget, the latter has the same
Qt::Window colour as the system colour scheme window background colour, if
you're using a dark terminal colour scheme and a light-coloured system colour
scheme, you could see some "flashing" when you close the last session with
e.g. Ctrl+D.

To fix this, in sessionFinished() check if TabbedViewContainer::count() is
1 (i.e. closing last tab/session), and emit the empty() signal in that case,
which is connected to MainwWindow::close(), then the order of tear down
becomes:
SessionController::sessionFinished()
~Session()
~MainWindow()
~ViewManager()
~TabbedViewContainer()
~TerminalDisplay()
FIXED-IN: 21.12
(cherry picked from commit bbec72250d080ce286a6762fb9beee4b6e7981c9)

M  +1    -1    src/MainWindow.cpp
M  +7    -0    src/ViewManager.cpp

https://invent.kde.org/utilities/konsole/commit/302c16791935cc3cf262aee355afce13d694b00f
Comment 8 Ahmad Samir 2021-08-13 13:08:46 UTC
OK, backported to 21.08 and 21.04; thanks for the reminder.
Comment 9 Dave Flogeras 2021-09-01 08:51:36 UTC
Hello again,

There seems to be a trickle down effect that made this bug come back.  After applying this patch, the Gentoo KDE team had bug reports related to side-effects and were subsequently patched.  Perhaps there is an interaction that causes the original patch to be invalid?  See the following comment for current patch-set

https://bugs.gentoo.org/807933#c7

Related bugs are linked to the above one.
Comment 10 Ahmad Samir 2021-09-01 09:24:20 UTC
Yeah, I saw that yesterday when I built from git master, I'll look into it.
Comment 11 Ahmad Samir 2021-09-01 09:42:27 UTC
FWIW, should be fixed by https://invent.kde.org/utilities/konsole/-/merge_requests/469
Comment 12 Dave Flogeras 2021-09-01 11:02:13 UTC
Thanks Ahmad, I can confirm that adding this patch to the existing patch-set mentioned in the Gentoo bug makes the behaviour go away again.
Comment 13 Ahmad Samir 2021-09-01 16:48:17 UTC
Git commit e693f2d7f1977ca227589154a5cd8c18d8ce44b7 by Ahmad Samir.
Committed on 01/09/2021 at 09:38.
Pushed by hindenburg into branch 'master'.

The default navigation method should be TabbedNavigation

TabbedNavigation is when we have a MainWindow, i.e. the typical use case;
whereas NoNavigation is when using Konsole Part. The code in Part calls
setNavigationMethod(NoNavigation), so things should work as before.

I made a wrong assumption that TabbedNavigation was already the default.

M  +1    -1    src/ViewManager.cpp

https://invent.kde.org/utilities/konsole/commit/e693f2d7f1977ca227589154a5cd8c18d8ce44b7
Comment 14 Kurt Hindenburg 2021-09-01 17:04:18 UTC
Git commit 58d526f83b924732b8c82306fcc177f0bbe63295 by Kurt Hindenburg, on behalf of Ahmad Samir.
Committed on 01/09/2021 at 16:57.
Pushed by hindenburg into branch 'release/21.08'.

The default navigation method should be TabbedNavigation

TabbedNavigation is when we have a MainWindow, i.e. the typical use case;
whereas NoNavigation is when using Konsole Part. The code in Part calls
setNavigationMethod(NoNavigation), so things should work as before.

I made a wrong assumption that TabbedNavigation was already the default.
(cherry picked from commit e693f2d7f1977ca227589154a5cd8c18d8ce44b7)

M  +1    -1    src/ViewManager.cpp

https://invent.kde.org/utilities/konsole/commit/58d526f83b924732b8c82306fcc177f0bbe63295
Comment 15 Timothy B 2021-09-02 23:18:20 UTC
I just upgraded KDE Gear (which includes Konsole) to 21.08.1 on Manjaro about an hour ago, but I'm still getting this exact annoying bug with Konsole, following the steps to reproduce in the bug description exactly. To make matters worse, the flash is a lot more noticable when you press Ctrl+D or even close the active with a middle click or through the right-click menu on Yakuake. I don't think this annoying bug is fully solved as of 21.08.1, which is said to have the fix backported per comment #14.
Comment 16 Ahmad Samir 2021-09-02 23:29:30 UTC
Unfortunately the fix didn't catch the git tagging of 20.08.1. The commit in comment#14 is basically a one-liner, so hopefully easy to backport.
Comment 17 Timothy B 2021-09-02 23:38:46 UTC
That's strange. In fact, that same patch was apparently backported to Konsole 21.08.0 on Arch and Manjaro a couple of weeks ago, and I never saw those annoying flashes go away after updating to the patched packages.
Comment 18 Ahmad Samir 2021-09-03 00:04:51 UTC
There are two commits, one in comment#7 and one in comment#14 with a couple of weeks between them.