D18805: Workaround for the bug 393630 - SystemTray part

2019-02-08 Thread Nathaniel Graham 
This revision was automatically updated to reflect the committed changes.
Closed by commit R120:6fcf9a5e03ba: Fix System Tray popup interactivity after 
echanging item visiblity (authored by trmdi, committed by ngraham).

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D18805?vs=51146=51190

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

AFFECTED FILES
  applets/systemtray/package/contents/ui/ConfigEntries.qml

To: trmdi, #plasma, ngraham, broulik, davidedmundson, anthonyfieroni, fvogt
Cc: plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D18805: Workaround for the bug 393630 - SystemTray part

2019-02-08 Thread Tranter Madi 
trmdi added a comment.


  In D18805#407599 , @fvogt wrote:
  
  > You can select "Abandon revision" as action.
  
  
  Done. Thank you. :)

REPOSITORY
  R120 Plasma Workspace

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

To: trmdi, #plasma, ngraham, broulik, davidedmundson, anthonyfieroni, fvogt
Cc: plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D18805: Workaround for the bug 393630 - SystemTray part

2019-02-08 Thread Fabian Vogt 
fvogt added a comment.


  In D18805#407542 , @trmdi wrote:
  
  > Use @davidedmundson 's approach.
  >
  > @ngraham Could you please help me to discard D18804 
 ? This patch does not require it anymore.
  
  
  You can select "Abandon revision" as action.
  
  > And land it.
  >  Thanks everyone !

REPOSITORY
  R120 Plasma Workspace

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

To: trmdi, #plasma, ngraham, broulik, davidedmundson, anthonyfieroni, fvogt
Cc: plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D18805: Workaround for the bug 393630 - SystemTray part

2019-02-07 Thread Tranter Madi 
trmdi added a comment.


  @davidedmundson
  
  Nice approach.
  I don't really understand the root cause though. So, could you please do it 
for me?
  
  Thank you!

REPOSITORY
  R120 Plasma Workspace

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

To: trmdi, #plasma, ngraham, broulik, davidedmundson, anthonyfieroni, fvogt
Cc: plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D18805: Workaround for the bug 393630 - SystemTray part

2019-02-07 Thread Anthony Fieroni 
anthonyfieroni added a comment.


  Ok, so @trmdi you can apply exactly David suggest and discard D18804 
, i've test it and it works. Push it in 
5.15 not 5.16 that's bug fix not feature. 5.12 can be also interested path as 
LTS version.

REPOSITORY
  R120 Plasma Workspace

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

To: trmdi, #plasma, ngraham, broulik, davidedmundson, anthonyfieroni, fvogt
Cc: plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D18805: Workaround for the bug 393630 - SystemTray part

2019-02-07 Thread David Edmundson 
davidedmundson requested changes to this revision.
davidedmundson added a comment.
This revision now requires changes to proceed.


  > I really don't know that sharing model between contexts cause a such 
behavior.
  
  Sorry, I used the wrong word. Sharing contexts is fine. Sharing across 
engines is problematic.
  
  QQmlData is shared between engines, but the relevant QObjectWrapper is
  not. Since 749a7212e903d8e8c6f256edb1836b9449cc7fe1 when a
  QObjectWrapper is deleted it resets the shared QQmlData propertyCache.
  
  Any code path on the first engine that use the propertycache need to rebuild 
it
  
  (In an ideal world we wouldn't have 2 engines, but that's a whole different 
topic)

REPOSITORY
  R120 Plasma Workspace

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

To: trmdi, #plasma, ngraham, broulik, davidedmundson, anthonyfieroni, fvogt
Cc: plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D18805: Workaround for the bug 393630 - SystemTray part

2019-02-07 Thread Anthony Fieroni 
anthonyfieroni added a comment.


  @davidedmundson it works. I really don't know that sharing model between 
contexts cause a such behavior. It's caused by some ref counting ? Did you know 
why, it's internal Qt issue that we can fix or it's expected ?

REPOSITORY
  R120 Plasma Workspace

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

To: trmdi, #plasma, ngraham, broulik, davidedmundson, anthonyfieroni, fvogt
Cc: plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D18805: Workaround for the bug 393630 - SystemTray part

2019-02-07 Thread David Edmundson 
davidedmundson added a comment.


  If the only usage is the SNI model lets just rebuild the model on the 
ConfigEntries side. 
  You're right that sharing objects between contexts seems to cause issues.
  
  It's backed by a datasource so all the heavy operations are already 
implicitly shared.
  
  We should only need
  
  PlasmaCore.DataSource {
 id: statusNotifierSource
 engine: "statusnotifieritem"
 interval: 0
 onSourceAdded: {
connectSource(source)
 }
 Component.onCompleted: {
 connectedSources = sources
 }
   }


PlasmaCore.SortFilterModel {
   id: statusNotifierModel
   sourceModel: PlasmaCore.DataModel {
   dataSource: statusNotifierSource
   }
   }
  
  and then port all the plasmoid.rootItem to just our local statusNotifierModel

REPOSITORY
  R120 Plasma Workspace

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

To: trmdi, #plasma, ngraham, broulik, davidedmundson, anthonyfieroni, fvogt
Cc: plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D18805: Workaround for the bug 393630 - SystemTray part

2019-02-07 Thread Tranter Madi 
trmdi added a comment.


  In D18805#406916 , @anthonyfieroni 
wrote:
  
  > I notice that something with model goes wrong. I've try to fix it in D18249 
 but unfortunately I don't see problem in.
  
  
  Does my workaround also fix your issue ?

REPOSITORY
  R120 Plasma Workspace

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

To: trmdi, #plasma, ngraham, broulik, davidedmundson, anthonyfieroni, fvogt
Cc: plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D18805: Workaround for the bug 393630 - SystemTray part

2019-02-07 Thread Anthony Fieroni 
anthonyfieroni added a comment.


  I notice that something with model goes wrong. I've try to fix it in D18249 
 but unfortunately I don't see problem in.

REPOSITORY
  R120 Plasma Workspace

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

To: trmdi, #plasma, ngraham, broulik, davidedmundson, anthonyfieroni, fvogt
Cc: plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D18805: Workaround for the bug 393630 - SystemTray part

2019-02-07 Thread Tranter Madi 
trmdi created this revision.
trmdi added reviewers: Plasma, ngraham, broulik.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
trmdi requested review of this revision.

REVISION SUMMARY
  - Require: https://phabricator.kde.org/D18804
  
  BUG: 393630

REPOSITORY
  R120 Plasma Workspace

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

AFFECTED FILES
  applets/systemtray/package/contents/ui/ConfigEntries.qml

To: trmdi, #plasma, ngraham, broulik
Cc: plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart