Abort updateWindowTitle and activeViewChanged if not changed.
ClosedPublic

Authored by rizzitello on Dec 31 2018, 2:03 AM.

Details

Summary
  • Prevent activeViewChanged from updating the window if the view is the same view (happens at least once when starting up)
  • Stop updateWindowTitle from updating the title if its not changed.

Diff Detail

Repository
R318 Dolphin
Branch
reduceRefreshCalls
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 7199
Build 7217: arc lint + arc unit
rizzitello created this revision.Dec 31 2018, 2:03 AM
Restricted Application added a project: Dolphin. · View Herald TranscriptDec 31 2018, 2:03 AM
Restricted Application added a subscriber: kfm-devel. · View Herald Transcript
rizzitello requested review of this revision.Dec 31 2018, 2:03 AM
rizzitello retitled this revision from Adjust window title on active window change Update View and title only if changed. to Abort updateWindowTitle and activeViewChanged if not changed..Dec 31 2018, 2:14 AM
rizzitello edited the summary of this revision. (Show Details)
rizzitello updated this revision to Diff 48428.Dec 31 2018, 2:15 AM
rizzitello edited the summary of this revision. (Show Details)
  • use newTitle to set title
broulik accepted this revision.Jan 10 2019, 1:22 PM
broulik added a subscriber: broulik.
broulik added inline comments.
src/dolphinmainwindow.cpp
1011–1014

const

This revision is now accepted and ready to land.Jan 10 2019, 1:22 PM
elvisangelaccio accepted this revision.Jan 13 2019, 9:28 AM
elvisangelaccio added inline comments.
src/dolphinmainwindow.cpp
975–977

This is a symptom that something else is wrong, but ok.

rizzitello updated this revision to Diff 49385.Jan 13 2019, 2:25 PM
rizzitello marked 2 inline comments as done.

Elvis' suggestion to use const QString for newTitle

rizzitello added inline comments.Jan 13 2019, 2:25 PM
src/dolphinmainwindow.cpp
975–977

This seams to be called twice when dolphin is just starting up and twice when you change tabs.
Digging around a little it seams to come from TabWidget where its sometimes emiting the view several times . I tried to stop it there, but it the mainwindow was still getting some extra calls sent to it. I think this fix may have to come in a future patch. Please Try my "testing" code below to see whats going on. Open dolphin and then open up tabs and when you change them you should see how often this is happening and being stopped by each layer.

To Test I Used this code here.

if (m_activeViewContainer == viewContainer) {
        static int calls = 1;
        qDebug() <<"ViewContainer Matches(" + QString::number(calls) + ")  " << sender();
        calls ++;
        return;
}

I also added this code to DolphinTabWidget::currentTabChanged() at line 330)

if(viewContainer->isActive()) {
    static int calls = 1;
    qDebug() <<"View Active(" + QString::number(calls) + ")  " << sender();
    calls ++;
    return;
}

Example output
Opening dolphin:

Trying to convert empty KLocalizedString to QString.
"ViewContainer Matches(1) " DolphinTabWidget(0x561f1877eb30, name = "tabWidget")

Create a new Tab:

"View Active(1) " DolphinTabWidget(0x561f1877eb30, name = "tabWidget")

Create a new Tab:

"View Active(2) " DolphinTabWidget(0x561f1877eb30, name = "tabWidget")

Switch to tab 1 (middle tab):

"View Active(3) " DolphinTabWidget(0x561f1877eb30, name = "tabWidget")

Switch to tab 2 (right most):

"View Active(4) " DolphinTabWidget(0x561f1877eb30, name = "tabWidget")

Switch to tab 0 (left most):

"ViewContainer Matches(2) " DolphinTabWidget(0x55c459fcec80, name = "tabWidget")
"ViewContainer Matches(3) " DolphinTabWidget(0x55c459fcec80, name = "tabWidget")
"ViewContainer Matches(4) " DolphinTabWidget(0x55c459fcec80, name = "tabWidget")

Every time you switch to the left most tab it has three of them .

Without the code in the TabWidget it got one ViewContainer Match for every tab crated and every tab i switched to.

What should I be using as a base for this code? Its currently based upon the Applications/18.12 branch

What should I be using as a base for this code? Its currently based upon the Applications/18.12 branch

master please

rizzitello updated this revision to Diff 49417.Jan 14 2019, 1:41 AM

Rebase on master

rizzitello updated this revision to Diff 49419.Jan 14 2019, 1:55 AM
  • Rebase to master

Is this good to merge?

elvisangelaccio accepted this revision.Jan 15 2019, 3:56 PM

Searchbar propagates events even it's not visible, you can add check for visibility in focusin event.

Searchbar propagates events even it's not visible, you can add check for visibility in focusin event.

Well that got me looking again and I think this time I solved it correctly.
The DolphinTabPage::viewChanged code was emiting the change even if it was the same item, I changed the logic to return first also for this function. Testing with my test code in DolphinMainWindow::viewChanged I no longer got any duplicate refreshes on start or otherwise. I tested with several tabs, search, filter and splits and by updating the directory in another program to see the dir updated in real time still.

I Found one case where it was still generating the duplicate view and that is when you close a tab between two other tabs . I don't think its a problem since it seams to be the correct behavior for this case

I'm Sorry to redo this yet again but this will be an overall better commit that solves the issue fully. Now . I will await your review.

rizzitello updated this revision to Diff 49635.Jan 16 2019, 2:12 PM
  • Better filter for activeView update

+1 it looks good to me.

elvisangelaccio accepted this revision.Jan 16 2019, 6:17 PM
  • Rebase preland
This revision was automatically updated to reflect the committed changes.