Fix files not being highlighted if directory of file is already open
Needs ReviewPublic

Authored by feverfew on Fri, Feb 14, 12:05 AM.

Details

Reviewers
broulik
Summary

In DolphinTabWidget::openFiles it assumes openDirectories will open new tabs, so it
only marks the URL selection for the newly opened tabs. This assumption is incorrect
as Dolphin might reuse tabs when the folder is already open.

Even then, markUrlsAsSelected does nothing when the folder is already opened.
The selection seems to only be used when the folder finishes loading, calls made after
that only change a variable without updating the actual selection.

A call to DolphinView::updateViewState() is required to restore the intended behaviour, so long as DolphinView::clearSelection() has been called on the relevant tab, otherwise updateViewState() is a no-op.

BUG: 417230
FIXED-IN: 19.12.3

Test Plan
  1. In any application that has an "open containing folder" select that option
  2. and then do the same for another file in the same folder

Diff Detail

Repository
R318 Dolphin
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 22535
Build 22553: arc lint + arc unit
feverfew created this revision.Fri, Feb 14, 12:05 AM
Restricted Application added a project: Dolphin. · View Herald TranscriptFri, Feb 14, 12:05 AM
Restricted Application added a subscriber: kfm-devel. · View Herald Transcript
feverfew requested review of this revision.Fri, Feb 14, 12:05 AM
broulik added inline comments.Fri, Feb 14, 9:09 AM
src/dolphintabwidget.cpp
246

I think we don't want to change existing tabs, only thew new ones.
You probably want to check

if (tabCount > oldTabCount) {
   // use this loop
} else {
  // set active on the tab it now selected for us
}
src/views/dolphinview.cpp
213 ↗(On Diff #75654)

updateViewState() resets state once it processed it, right? i.e. I want to avoid changing between views and tabs to randomly scroll back to whatever file it was once asked to highlight.

I don't like abusing this method for this.

feverfew updated this revision to Diff 75768.Sun, Feb 16, 2:39 PM
  • Actually fix the bug
feverfew updated this revision to Diff 75774.Sun, Feb 16, 3:19 PM
  • Remote unnecessary newline
feverfew edited the summary of this revision. (Show Details)Sun, Feb 16, 4:34 PM
feverfew edited the test plan for this revision. (Show Details)