D18598: Defer initial positions apply until listing is complete

[Eike Hein]

This revision was automatically updated to reflect the committed changes.
Closed by commit R119:aaebb51077ae: Defer initial positions apply until listing 
is complete (authored by hein).

CHANGED PRIOR TO COMMIT
  https://phabricator.kde.org/D18598?vs=50509&id=50545#toc

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D18598?vs=50509&id=50545

REVISION DETAIL
  https://phabricator.kde.org/D18598

AFFECTED FILES
  containments/desktop/package/contents/ui/FolderView.qml
  containments/desktop/package/contents/ui/main.qml
  containments/desktop/plugins/folder/positioner.cpp
  containments/desktop/plugins/folder/positioner.h

To: hein, #plasma, davidedmundson, chinmoyr
Cc: plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D18598: Defer initial positions apply until listing is complete

[Eike Hein]

hein added a comment.


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

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D18598

To: hein, #plasma, davidedmundson, chinmoyr
Cc: plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D18598: Defer initial positions apply until listing is complete

[Eike Hein]

hein updated this revision to Diff 50509.
hein added a comment.


  Revert an unintended change, fixes positionertest

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D18598?vs=50506&id=50509

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D18598

AFFECTED FILES
  containments/desktop/package/contents/ui/FolderView.qml
  containments/desktop/package/contents/ui/main.qml
  containments/desktop/plugins/folder/positioner.cpp
  containments/desktop/plugins/folder/positioner.h

To: hein, #plasma, davidedmundson, chinmoyr
Cc: plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D18598: Defer initial positions apply until listing is complete

[Eike Hein]

hein updated this revision to Diff 50506.
hein added a comment.


  Add missing include

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D18598?vs=50502&id=50506

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D18598

AFFECTED FILES
  containments/desktop/package/contents/ui/FolderView.qml
  containments/desktop/package/contents/ui/main.qml
  containments/desktop/plugins/folder/positioner.cpp
  containments/desktop/plugins/folder/positioner.h

To: hein, #plasma, davidedmundson, chinmoyr
Cc: plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D18598: Defer initial positions apply until listing is complete

[Eike Hein]

hein updated this revision to Diff 50502.
hein added a comment.


  Signal-slot hygiene

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D18598?vs=50498&id=50502

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D18598

AFFECTED FILES
  containments/desktop/package/contents/ui/FolderView.qml
  containments/desktop/package/contents/ui/main.qml
  containments/desktop/plugins/folder/positioner.cpp
  containments/desktop/plugins/folder/positioner.h

To: hein, #plasma, davidedmundson, chinmoyr
Cc: plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D18598: Defer initial positions apply until listing is complete

[Eike Hein]

hein updated this revision to Diff 50498.
hein added a comment.


  Reorder abort checks and unset m_deferApplyPositions if going for a reset

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D18598?vs=50490&id=50498

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D18598

AFFECTED FILES
  containments/desktop/package/contents/ui/FolderView.qml
  containments/desktop/package/contents/ui/main.qml
  containments/desktop/plugins/folder/positioner.cpp
  containments/desktop/plugins/folder/positioner.h

To: hein, #plasma, davidedmundson, chinmoyr
Cc: plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D18598: Defer initial positions apply until listing is complete

[Eike Hein]

hein added a comment.


  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.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D18598

To: hein, #plasma, davidedmundson, chinmoyr
Cc: plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D18598: Defer initial positions apply until listing is complete

[David Edmundson]

davidedmundson added a comment.


  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.

INLINE COMMENTS

> positioner.cpp:82
>  if (m_folderModel) {
>  disconnectSignals(m_folderModel);
>  }

you're not disconnecting newly added connections

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D18598

To: hein, #plasma, davidedmundson, chinmoyr
Cc: plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D18598: Defer initial positions apply until listing is complete

[Eike Hein]

hein created this revision.
hein added reviewers: Plasma, davidedmundson, chinmoyr.
Herald added a project: Plasma.
hein requested review of this revision.

REVISION 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

REPOSITORY
  R119 Plasma Desktop

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D18598

AFFECTED FILES
  containments/desktop/package/contents/ui/FolderView.qml
  containments/desktop/package/contents/ui/main.qml
  containments/desktop/plugins/folder/positioner.cpp
  containments/desktop/plugins/folder/positioner.h

To: hein, #plasma, davidedmundson, chinmoyr
Cc: plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart