Fix an issue with a new tab focus
ClosedPublic

Authored by AndreyYashkin on Jul 10 2019, 6:16 PM.

Details

Summary

When opening a new tab in the background and switching to that tab the focus is set on the location bar instead of the files view. If you switch back to the original tab and then to the new tab again focus will be set on the files view. The problem is caused by creation of DolphinTabPage in an active state which leads to skipping by return in DolphinView::setActive(bool active) without setting the focus on the view. This patch fixes this defect.

BUG: 407604

Test Plan
  1. Open a new tab in the background
  2. Switch to the new tab
  3. Check current focus widget by the up/down arrows on the keyboard with and without changes.

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 created this revision.Jul 10 2019, 6:16 PM
Restricted Application added a subscriber: kfm-devel. · View Herald TranscriptJul 10 2019, 6:16 PM
AndreyYashkin requested review of this revision.Jul 10 2019, 6:16 PM
AndreyYashkin added a reviewer: Dolphin.
ngraham accepted this revision.Jul 11 2019, 2:35 AM
ngraham added subscribers: elvisangelaccio, ngraham.

Yep, works and the change makes sense to me. @elvisangelaccio?

This revision is now accepted and ready to land.Jul 11 2019, 2:35 AM
elvisangelaccio requested changes to this revision.Jul 14 2019, 8:58 PM

Hmm, this feels like a work-around because we initialize m_active(true) everywhere else (DolphinView, DolphinSearchBox).

If possible it should be initialized to false only if the tab page is a background tab.

This revision now requires changes to proceed.Jul 14 2019, 8:58 PM

Equivalent solution

It does not look correct either, the problem is that the focus on location bar is delayed and setActive(false) isn't applied, this is workaround. But we have problems on that before, exactly that part with focusIn on location bar. You should correct there but it can break something other.

It does not look correct either, the problem is that the focus on location bar is delayed and setActive(false) isn't applied, this is workaround. But we have problems on that before, exactly that part with focusIn on location bar. You should correct there but it can break something other.

When tab is switched, DolphinTabWidget::currentTabChanged deactivates last-viewed tab page and activates new current tab, and there should be only one active tab at same moment. The problem is that DolphinTabWidget::openNewTab creates tabs in active state and now there several tabs in active state, which don't get the focus on the files view after switching on them first time, because DolphinView::setActive doesn't do anything if new active state(true) is equal to old state(true), while it should set focus on the view.

elvisangelaccio accepted this revision.Sun, Jul 28, 2:19 PM

I like this solution better. I'll push on master only for now, and we'll see if we break something ;)

This revision is now accepted and ready to land.Sun, Jul 28, 2:19 PM
This revision was automatically updated to reflect the committed changes.