D28658: [krdb] Drop GTK2 colour exporting

[Fabian Vogt]

fvogt added a comment.


  > FIXED-IN: 5.19.0 (or should we consider this a bugfix and land it on the 
stable branch?)
  
  At least in openSUSE we'll backport it to Plasma 5.18, but it might not be 
the kind of behaviour change desired in 5.18 at this point in time...

REPOSITORY
  R119 Plasma Desktop

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

To: cblack, #plasma, ngraham, davidedmundson
Cc: gikari, fvogt, ngraham, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, 
cblack, jraleigh, zachus, fbampaloukas, ragreen, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28658: [krdb] Drop GTK2 colour exporting

[Carson Black]

This revision was automatically updated to reflect the committed changes.
Closed by commit R119:2fb0bf85fe63: [krdb] Drop GTK2 colour exporting (authored 
by cblack).

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28658?vs=79579&id=80406

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

AFFECTED FILES
  kcms/krdb/krdb.cpp

To: cblack, #plasma, ngraham, davidedmundson
Cc: gikari, fvogt, ngraham, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, 
cblack, jraleigh, zachus, fbampaloukas, ragreen, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28658: [krdb] Drop GTK2 colour exporting

[Mikhail Zolotukhin]

gikari resigned from this revision.
gikari added a comment.


  No objections from me.

REPOSITORY
  R119 Plasma Desktop

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

To: cblack, #plasma, ngraham, davidedmundson
Cc: gikari, fvogt, ngraham, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, 
cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28658: [krdb] Drop GTK2 colour exporting

[Carson Black]

cblack added inline comments.

INLINE COMMENTS

> gikari wrote in krdb.cpp:642
> Is this somewhat relevant to GTK?

Pretty sure it's writing the xrdb there.

REPOSITORY
  R119 Plasma Desktop

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

To: cblack, #plasma, ngraham, gikari, davidedmundson
Cc: gikari, fvogt, ngraham, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, 
cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28658: [krdb] Drop GTK2 colour exporting

[Mikhail Zolotukhin]

gikari added a comment.


  We also need to remove the remaining GTK stuff from here once D28070 
 and D28072 
 are landed. Does anyone else wants to test 
the later?

INLINE COMMENTS

> krdb.cpp:642
>// Export colors to non-(KDE/Qt) apps (e.g. Motif, GTK+ apps)
>if (exportColors)
>{

Is this somewhat relevant to GTK?

REPOSITORY
  R119 Plasma Desktop

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

To: cblack, #plasma, ngraham, gikari, davidedmundson
Cc: gikari, fvogt, ngraham, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, 
cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28658: [krdb] Drop GTK2 colour exporting

[Nathaniel Graham]

ngraham accepted this revision.
ngraham added reviewers: gikari, davidedmundson. ngraham added 1 blocking 
reviewer(s): Plasma.
ngraham added a subscriber: gikari.
ngraham added a comment.


  Conceptually this makes sense, and the support for this doesn't really work 
anymore and is in fact causing 412331 (the bug got introduced when this code 
started working again when recoloration support went in).
  
  Let's make sure @gikari and the #plasma 
 folks are okay with this though.

REPOSITORY
  R119 Plasma Desktop

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

To: cblack, #plasma, ngraham, gikari, davidedmundson
Cc: gikari, fvogt, ngraham, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, 
cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28658: [krdb] Drop GTK2 colour exporting

[Carson Black]

cblack updated this revision to Diff 79579.
cblack added a comment.


  Drop the exportColors argument

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28658?vs=79577&id=79579

BRANCH
  cblack/drop-gtk2-colors (branched from master)

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

AFFECTED FILES
  kcms/krdb/krdb.cpp

To: cblack, #plasma
Cc: fvogt, ngraham, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28658: [krdb] Drop GTK2 colour exporting

[Nathaniel Graham]

ngraham added a comment.


  Probably needs some inline documentation here (as well as in the header file) 
that the `exportColors` arg now is does a no-op. Or can we even remove it 
entirely?

REPOSITORY
  R119 Plasma Desktop

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

To: cblack, #plasma
Cc: ngraham, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28658: [krdb] Drop GTK2 colour exporting

[Carson Black]

cblack created this revision.
cblack added a reviewer: Plasma.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
cblack requested review of this revision.

REVISION SUMMARY
  With the vast supermajority of modern GTK2 themes using pixmaps, exporting 
colours
  is only likely to result in breakage.

REPOSITORY
  R119 Plasma Desktop

BRANCH
  cblack/drop-gtk2-colors (branched from master)

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

AFFECTED FILES
  kcms/krdb/krdb.cpp

To: cblack, #plasma
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart