Fix an issue with focus lost after closing terminal panel
ClosedPublic

Authored by AndreyYashkin on Jul 12 2019, 1:01 PM.

Details

Summary

After leaving terminal with Ctrl-D or exit commands the input focus isn't set back to the folder view. The problem appears, because TerminalPanel::isHiddenInVisibleWindow
returns not what it supposed to return. I moved unwanted checks from it inside TerminalPanel::dockVisibilityChanged

BUG: 407979

Test Plan
  1. Open Dolphin
  2. Press F4 to open the terminal panel
  3. Type exit<Enter> or press Ctrl-D
  4. Check current focus widget

Diff Detail

Repository
R318 Dolphin
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
AndreyYashkin requested review of this revision.Jul 12 2019, 1:01 PM
AndreyYashkin created this revision.

This issue was supposed to be fixed by commit 4e40fe810d324 but it seems it got reintroduced in the past months.

Could you try to git bisect it to figure out when this happend?

src/dolphinmainwindow.cpp
754

This looks like an unrelated change, if it's not can you explain why we need it?

AndreyYashkin marked an inline comment as done.EditedJul 15 2019, 1:08 PM

This issue was supposed to be fixed by commit 4e40fe810d324 but it seems it got reintroduced in the past months.

Could you try to git bisect it to figure out when this happend?

It seems that it was not fixed. I can reproduce the problem at 4e40fe810d324.

src/dolphinmainwindow.cpp
754

m_activeViewContainer is NULL when Dolphin starts and without this change app crashes.

AndreyYashkin marked an inline comment as done.Sun, Jul 21, 8:41 PM

Friendly ping!

I'd still like to understand what happend at the change in 4e40fe810d324.

@ngraham Any idea?

I'd still like to understand what happend at the change in 4e40fe810d324.

@ngraham Any idea?

@elvisangelaccio, I read the description of change in 4e40fe810d324 and bug 298467, which was fixed in it more carefull and understood that the change was supposed to introduce the feature of setting the focus back to the active view. However, it seems that Ctrl-D or exit command cases were not tested good enough.

elvisangelaccio accepted this revision.Sun, Aug 11, 11:22 AM

I'd still like to understand what happend at the change in 4e40fe810d324.

@ngraham Any idea?

@elvisangelaccio, I read the description of change in 4e40fe810d324 and bug 298467, which was fixed in it more carefull and understood that the change was supposed to introduce the feature of setting the focus back to the active view. However, it seems that Ctrl-D or exit command cases were not tested good enough.

Right. The fix looks correct to me, going to push it to master (with an improved commit message).

src/dolphinmainwindow.cpp
754

You're right, it was a problem also before and it was working just by luck...

This revision is now accepted and ready to land.Sun, Aug 11, 11:22 AM
This revision was automatically updated to reflect the committed changes.