Differential D18598

Defer initial positions apply until listing is complete
ClosedPublic
Actions

Authored by hein on Jan 29 2019, 5:09 PM.

Details

Reviewers
davidedmundson
chinmoyr
Group Reviewers
Plasma
Commits
R119:aaebb51077ae: Defer initial positions apply until listing is complete
Summary

This fixes the infamous "desktop positions partially scramble on reboot"
bug that occurs when KDirLister completes listing in multiple model
transactions.

This also:

  • Disallows moves and drops while listing, for extra safety.
  • Cleans up wonky old defer-sometimes code that made little sense.
  • Removes a cache for lastRow() that was never actually used.

BUG:354802

Diff Detail

Repository
R119 Plasma Desktop
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
hein created this revision.Jan 29 2019, 5:09 PM
Restricted Application added a project: Plasma. · View Herald TranscriptJan 29 2019, 5:09 PM
hein requested review of this revision.Jan 29 2019, 5:09 PM
davidedmundson added a comment.Jan 29 2019, 5:41 PM

If the QML code calls setPositions({}) and when we apply m_positions.size() < 5
m_deferApplyPositions gets left on true forever. Is that ok?

Other than that, looks good.

containments/desktop/plugins/folder/positioner.cpp
82

you're not disconnecting newly added connections

hein added a comment.Jan 29 2019, 6:15 PM
In D18598#401848, @davidedmundson wrote:

If the QML code calls setPositions({}) and when we apply m_positions.size() < 5
m_deferApplyPositions gets left on true forever. Is that ok?

It's not, good catch. Updating in a sec.

hein updated this revision to Diff 50498.Jan 29 2019, 6:17 PM

Reorder abort checks and unset m_deferApplyPositions if going for a reset

hein updated this revision to Diff 50502.Jan 29 2019, 6:26 PM

Signal-slot hygiene

hein updated this revision to Diff 50506.Jan 29 2019, 6:32 PM

Add missing include

hein updated this revision to Diff 50509.Jan 29 2019, 7:04 PM

Revert an unintended change, fixes positionertest

hein added a comment.Jan 29 2019, 7:08 PM

Sorry for the noise. Please review again (I'm off till tomorrow).

davidedmundson accepted this revision.Jan 30 2019, 2:02 AM
This revision is now accepted and ready to land.Jan 30 2019, 2:02 AM
Closed by commit R119:aaebb51077ae: Defer initial positions apply until listing is complete (authored by hein). · Explain WhyJan 30 2019, 9:54 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 50545

View Options

containments/desktop/package/contents/ui/FolderView.qml

Loading...
View Options

containments/desktop/package/contents/ui/main.qml

Loading...
View Options

containments/desktop/plugins/folder/positioner.h

Loading...
View Options

containments/desktop/plugins/folder/positioner.cpp

Loading...