Differential D12446

Update window title after closing split view
ClosedPublic
Actions

Authored by rjennings on Apr 22 2018, 1:05 PM.

Details

Reviewers
ngraham
elvisangelaccio
Group Reviewers
Dolphin
Commits
R318:78c8b36dd84a: Update window title after closing split view
Summary

Posted this to bugs.kde.org. Just copy/pasting what I said there.

In dolphintabpage.cpp:

Through the connections set up in createViewContainer(), m_primaryViewContainer->setActive(true), at line 98, ends up triggering slotViewActivated().

Because m_primaryViewActive is true, oldActiveView is not set to the view needed in this particular situation. Both oldActiveView and newActiveView end up pointing to the same view and the connections are not set up.

Adding

m_primaryViewActive = false;

inside the if clause at line 93 seems to fix it?

BUG: 385111

Test Plan
  1. Open split view
  2. Make left panel active
  3. Close split view
  4. Navigate to various directories

Window title now updates to show current directory

Also tried with multiple tabs/combinations

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.
rjennings requested review of this revision.Apr 22 2018, 1:05 PM
rjennings created this revision.
ngraham added a subscriber: ngraham.Apr 22 2018, 5:42 PM

Thanks for the patch! Please add "BUG: <number of bug> somewhere to the Summary text. https://community.kde.org/Infrastructure/Phabricator#Add_special_keywords

Also please update the Test Plan section to indicate how you tested the patch.

rjennings edited the summary of this revision. (Show Details)Apr 22 2018, 7:28 PM
rjennings edited the test plan for this revision. (Show Details)
ngraham accepted this revision.Apr 22 2018, 10:52 PM

Thanks, this fixes the issue for me! I played around with split view a lot and wasn't able to find any regressions. Other Dolphin folks, does this look sane?

Robert, to land this, we'll need an email address for you. Which one is best?

This revision is now accepted and ready to land.Apr 22 2018, 10:52 PM
rjennings added a comment.Apr 23 2018, 2:55 PM

Cool. Please use rjennings.kde at gmail.com

elvisangelaccio added a subscriber: elvisangelaccio.Apr 23 2018, 9:46 PM

From a quick look it seems there is something strange going on here. I'll investigate in the next days, please don't push it yet.

ngraham added a comment.Apr 23 2018, 10:26 PM

Right. I'll wait.

elvisangelaccio accepted this revision.Apr 25 2018, 10:43 AM

This is indeed the proper fix, since m_primaryViewActive will be set to true again in slotViewActivated(). I also added a test case for this bug.

Thanks for the patch!

Closed by commit R318:78c8b36dd84a: Update window title after closing split view (authored by rjennings, committed by elvisangelaccio). · Explain WhyApr 25 2018, 10:50 AM
This revision was automatically updated to reflect the committed changes.
elvisangelaccio mentioned this in R318:74c2d4ea2d25: Add failing test case for bug #385111.

Revision Contents
Changeset List

PathPackages
Msrc/dolphintabpage.cpp (1 line)

Diff 33058

View Options

src/dolphintabpage.cpp

Loading...