Re: Review Request 126308: KPushButton: respect SH_DialogButtonBox_ButtonsHaveIcons, also when drawing

[René J . V . Bertin]

> On Dec. 10, 2015, 11:11 p.m., Thomas Lübking wrote:
> > 1. What tells you that this is a dialog buttonbox pushbutton?
> > 2. What happens if the button has no text?
> > 
> > 
> > The bug is in QDialogButtonBox (or rather the K variant, 
> > QDialogButtonBoxPrivate::createButton() seems to incorporate the hint 
> > correctly)
> > 
> > 
> > [KDE] has "ShowIconsOnPushButtons=false" in kdeglobals which was 
> > interpreted by KPushButton _and_ kstyle for the hint, but the hint only 
> > covers Q'DialogButtonBox'es - there's simply no global rule for this like 
> > AA_DontShowIconsInMenus
> > 
> > => KDialogButtonBox shall respect the hint for its buttons (there're two 
> > special creation routines).
> > 
> > For the rest, the platform plugin ideally picks the kdeglobals setting and 
> > exports it to the application object (dyn property?) where the style can 
> > pick it and incorporate it into its calculations (ie. if no icons are 
> > wanted and there's text or arrow, omit the icon in size calculation and 
> > painting)
> > 
> > "Fixing" that in deprecated KPushButton doesn't really fix anything. We'll 
> > face the mix we had, just that users of QPushButton were far less prone to 
> > pass them an icon in pre-KF5 times.
> > 
> > Please also attach Hugo Pereira Da Costa.
> 
> René J.V. Bertin wrote:
> 1: why should one care? It is said nowhere that the setting defined as 
> "show icons on buttons" in `systemsettings(5)` applies only to dialogs. 
> Rather, the tooltip says "when this is enabled, KDE applications will show 
> icons alongside [sic!] some important buttons".
> In other words, when "this" is *not* enabled, there should be no icons, 
> period.
> I have found no sign in the code that the ShowIconsOnPushButtons hint is 
> to be used only for dialogs; `SH_DialogButtonBox_ButtonsHaveIcons` indeed 
> carries the suggestion in its name but I would not be surprised if Qt really 
> thinks of it in a more general sense. Probably also because the notion of 
> what a dialog is has become a lot vaguer.
> 
> And that also happens to be what Qt does; buttons show their icons on 
> Linux (and other Unix variants?) but on OS X or MS Windows displaying of 
> those icons is deactivated unless you use a style that enables it. In fact, 
> the default setting for `SH_DialogButtonBox_ButtonsHaveIcons` is false except 
> in the generic Unix theme (= it does work globally like 
> `AA_DontShowIconsInMenus` everywhere else).
> 
> 2: a user who indicates s/he doesn't want to see icons will get an empty 
> button. That's also what can happen with QPushButton, and app developers have 
> to take this into consideration. Cf. toolbars (and Qt's position on the use 
> of "texted separators").
> I don't think I've ever come across a standard button showing only an 
> icon, except possibly the arrow button next to the progress indicators in 
> KMail and KDevelop.
> 
> As to fixing it here: as it turns out, "here" is the main source for 
> annoying icons rearing their silly heads on buttons on my screen ;) and it 
> was also something of a challenge to understand why they kept appearing 
> despite the fact that all code appeared to return the value of 
> `ShowIconsOnPushButtons`. Deprecated or not, it doesn't look like all 
> applications are going to stop using it anytime soon.
> 
> I looked into fixing the issue in KDialogButtonBox but saw that it does 
> nothing to override the `ShowIconsOnPushButtons` setting. The only way to 
> respect the setting through that class (or a modern equivalent) would be to 
> set an empty icon if `ShowIconsOnPushButtons=false`. That introduces another 
> regression: changes in this setting are supposed to be reflected by running 
> applications without requiring a restart (or a recreation of dialogs). If it 
> were just me I'd decree that buttons can have either text or an icon, but 
> right now we have to make do with this mixed situation.
> 
> I don't mind making this an OS X (and MS Windows) specific modification, 
> of course, but on those platforms
> 
> Thomas Lübking wrote:
> > 1: why should one care?
> 
> Because, as explained, that is what the hint says. Nothing else.
> 
> > I have found no sign in the code that the ShowIconsOnPushButtons hint 
> is to be used only for dialogs
> 
> No, but it's been used to feed SH_DialogButtonBox_ButtonsHaveIcons *AND* 
> KPushButton.
> ShowIconsOnPushButtons implied SH_DialogButtonBox_ButtonsHaveIcons but 
> NOT vv.
> 
> The approach is wrong, since you're abusing the hint for generalisation.
> 
> > but on OS X or MS Windows
> 
> ... Qt uses native elements which might simply globally downforce the 
> pushbutton icon nonsense (as could any style - I was more than once close to 
> doing that in virtuality)
> Eg. Breeze might do that on favor of the HIG, but that's not relevant 
> here.
> 
> Downforcing in KPushButton means to 

Icons Missing

[Simon Lees]

Hi,

Firstly sorry for breaking threading (I'm not on list and couldn't find 
a easy way to reply, please cc me in replies).


I am the enlightenment maintainer for openSUSE and I am hitting the same 
issues running many of the programs I use daily under enlightenment 
(Dolphin, kate, ...). In addition to missing icons any changes to the 
"Color Scheme" made in systemsettings5 arn't applied. Both of these 
worked in KDE4 and now feel like a major regression.




El Sunday 06 December 2015, a les 13:56:19, Thorsten Zachmann va escriure:

/Hello all, />>//>>/I use a separate user for running calligra. I use ssh -X to login from my 
/>>/normal desktop user to my kde running user. However when I start any />>/kde application I 
have no icons. />

You are not using any desktop environment thus the Qt defaults to the generic
Unix icon theme, i.e. hicolor.

Blame Linux for not having a cross desktop environment way to define what is
the icon theme to use.



That indeed would be nice, but I doubt there would also be a cross DE way of
setting the KDE/Qt Color scheme :-P, although enlightenment does have a way
of setting the gtk theme / icon theme.

While such a thing does not exist it would be really nice if it used the color
scheme / icon themes defined in systemsettings5, so that its changeable, us
people who use non standard DE's are used to going and loading up
systemsettings to change the look of Qt based apps and using something like
lxapperance to change gtk based ones. Not being able to set a icon theme or
color scheme is a major regression.


Cheers,
  Albert

/With strace I can see it searches for />>/icons in the hicolor folder instead of breeze. />>/With the help of David Faure I found out the the icons are shown as 
expected />>/when I call />>//>>/export KDE_FULL_SESSION=true />>//>>/before starting the application. Unfortunately I can't define this 
globally as it will break xdg-utils and who knows what else, so I guess 
i'm left launching from a terminal or hacking the .desktop files for the 
kde apps I use to export it before running. />>//>>/I think using an application via ssh -X is used quite often and it should />>/work out of the box with the need of setting any special export. Maybe you />>/have an idea on how the behaviour can be improved. />>//>>/Please CC me as I'm not subscribed to the list. />>//>>/Thorsten Zachmann /

Cheers

Simon Lees

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

system tray test?

[René J . V . Bertin]

Hi,

Does KF5 provide any classes above Qt's for putting up and controlling an icon 
(with/out menu) in the "system tray"?
If so, is there a simple test app I can try?

Reason I'm asking: kwalletmanager5 isn't showing a systray interface like 
kwalletmanager(4) does. Not on OS X (where Qt5's systray does work), and on 
Linux under a KDE4 desktop I'm not sure what I'm getting.

Thanks,
René
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: RFC: split platformtheme plugin from frameworkintegration and move to kde/workspace

[Weng Xuetian]

Hi Mark,

Sorry to jump in the discussion. It's totally fine you found
frameworkintegration currently also useful for other desktop. But in
this specific case, if you want to achieve the same feature in some
other desktop, it would be better to make a fork of
frameworkintegration.

Currently frameworkintegration is already coupled with plasma desktop,
e.g. using kde configured icon theme, need env variable set by
startkde/startplasmacompositor to make it work, and there could be
even more in the future. And that's not a wrong thing, it is designed
to integration qt application with plasma desktop.

Making a fork and then so you can do whatever yon want. E.g. if LxQt
found kio file dialog useful, they could also keep that part of code,
and maybe read icon theme configuration from lxqt's config file
instead of kde one's.

The things you ask doesn't really need to happen under plasma's
frameworkintegration. Libraries in frameworks are more independent to
the desktop, one can make use of them to implement their own
platformtheme plugin for Qt if they found they are useful.

You probably think that Qt should split the platform theme in to many
small plugin that each one only handles a single aspect of
integration, but that's not how it works right now and probably isn't
worth the effort to do so.

Regards,
Xuetian



On Thu, Dec 10, 2015 at 5:39 AM, Mark Gaiser  wrote:
>
>
> On Thu, Dec 10, 2015 at 10:20 AM, Martin Graesslin 
> wrote:
>>
>> On Thursday, December 10, 2015 9:54:15 AM CET Mark Gaiser wrote:
>> > On Thu, Dec 10, 2015 at 8:07 AM, Martin Graesslin 
>> >
>> > wrote:
>> > > On Wednesday, December 9, 2015 4:03:24 PM CET Aleix Pol wrote:
>> > > > On Wed, Dec 9, 2015 at 3:56 PM, Mark Gaiser 
>> > > > wrote:
>> > > > > On Wed, Dec 9, 2015 at 8:24 AM, Martin Graesslin
>> > > > > 
>> > >
>> > > wrote:
>> > > > >> On Tuesday, December 8, 2015 6:03:47 PM CET Mark Gaiser wrote:
>> > > > >> > I thought the frameworkintegration plugin was exactly that and
>> > >
>> > > usable
>> > >
>> > > > >> > for
>> > > > >> > any platform if they wish to use it.
>> > > > >> > Or is my assumption wrong and is it really only for Plasma and
>> > >
>> > > should
>> > >
>> > > > >> > others stay away from it?
>> > > > >>
>> > > > >> well obviously it's only for plasma as it's bound to env
>> > > > >> variables
>> > >
>> > > set by
>> > >
>> > > > >> startkde. And in 4.x time the qpt plugin was in kde-workspace
>> > > > >> repo,
>> > >
>> > > see:
>> > >
>> > >
>> > >
>> > > https://quickgit.kde.org/?p=kde-workspace.git=blob=4f67cc55104fe1081b
>> > >
>> > >
>> > > 05d381e9516e0215f8e24a=1b97d4427257120e305408404bff5ec6be0b65a9=qgui
>> > >
>> > > > >> platformplugin_kde %2Fqguiplatformplugin_kde.cpp
>> > > > >>
>> > > > >> > My assumption can very well be wrong, but then i think we need
>> > > > >> > to
>> > >
>> > > have
>> > >
>> > > > >> > a
>> > > > >> > "base" frameworkintegration that every app dev can use with or
>> > >
>> > > without
>> > >
>> > > > >> > plasma. And a plasma specific version that integrates more in
>> > >
>> > > plasma i
>> > >
>> > > > >> > suppose.
>> > > > >>
>> > > > >> I don't think it's anything an app developer should care about.
>> > > > >> It's
>> > > > >> integration, that's not something the app developer picks -
>> > > > >> otherwise
>> > >
>> > > the
>> > >
>> > > > >> app
>> > > > >> breaks on integrating with other platforms.
>> > > > >>
>> > > > >> > I don't care for that either. It's logical and to be expected.
>> > > > >> > I do care when i want to use the KF5 filedialog and need to
>> > > > >> > install
>> > > > >> > plasma
>> > > > >> > (which has absolutely nothing to do with the dialog) just to
>> > > > >> > get
>> > > > >> > it.
>> > > > >> > With "use" i mean the file open dialog, not the ones you can
>> > > > >> > just
>> > >
>> > > call
>> > >
>> > > > >> > from
>> > > > >> > the C++ side of things.
>> > > > >> >
>> > > > >> > And i definitely do not want to make a QPA just to have that
>> > >
>> > > working.
>> > >
>> > > > >> Well you have to. The file dialog is part of integration. If you
>> > > > >> want
>> > >
>> > > to
>> > >
>> > > > >> have
>> > > > >> a specific integration you need to provide a QPT (not QPA)
>> > > > >> plugin.
>> > > > >> Application
>> > > > >> developers must keep away from that.
>> > > > >>
>> > > > >> Please also read up on the topic of why KFileDialog got removed
>> > > > >> from
>> > >
>> > > our
>> > >
>> > > > >> API.
>> > > > >>
>> > > > >> Cheers
>> > > > >> Martin
>> > > > >
>> > > > > I see what you mean, i understand your opinion, but... I just
>> > > > > don't
>> > >
>> > > like
>> > >
>> > > > > it
>> > > > > for all the reasons given earlier.
>> > > > > I might be a minority here, not many people are responding besides
>> > >
>> > > Aleix
>> > >
>> > > > > and myself.
>> > > > >
>> > > > > Lets both take a step back and let some other 

Re: Review Request 126308: KPushButton: respect SH_DialogButtonBox_ButtonsHaveIcons, also when drawing

[Thomas Lübking]

> On Dec. 10, 2015, 10:11 p.m., Thomas Lübking wrote:
> > 1. What tells you that this is a dialog buttonbox pushbutton?
> > 2. What happens if the button has no text?
> > 
> > 
> > The bug is in QDialogButtonBox (or rather the K variant, 
> > QDialogButtonBoxPrivate::createButton() seems to incorporate the hint 
> > correctly)
> > 
> > 
> > [KDE] has "ShowIconsOnPushButtons=false" in kdeglobals which was 
> > interpreted by KPushButton _and_ kstyle for the hint, but the hint only 
> > covers Q'DialogButtonBox'es - there's simply no global rule for this like 
> > AA_DontShowIconsInMenus
> > 
> > => KDialogButtonBox shall respect the hint for its buttons (there're two 
> > special creation routines).
> > 
> > For the rest, the platform plugin ideally picks the kdeglobals setting and 
> > exports it to the application object (dyn property?) where the style can 
> > pick it and incorporate it into its calculations (ie. if no icons are 
> > wanted and there's text or arrow, omit the icon in size calculation and 
> > painting)
> > 
> > "Fixing" that in deprecated KPushButton doesn't really fix anything. We'll 
> > face the mix we had, just that users of QPushButton were far less prone to 
> > pass them an icon in pre-KF5 times.
> > 
> > Please also attach Hugo Pereira Da Costa.
> 
> René J.V. Bertin wrote:
> 1: why should one care? It is said nowhere that the setting defined as 
> "show icons on buttons" in `systemsettings(5)` applies only to dialogs. 
> Rather, the tooltip says "when this is enabled, KDE applications will show 
> icons alongside [sic!] some important buttons".
> In other words, when "this" is *not* enabled, there should be no icons, 
> period.
> I have found no sign in the code that the ShowIconsOnPushButtons hint is 
> to be used only for dialogs; `SH_DialogButtonBox_ButtonsHaveIcons` indeed 
> carries the suggestion in its name but I would not be surprised if Qt really 
> thinks of it in a more general sense. Probably also because the notion of 
> what a dialog is has become a lot vaguer.
> 
> And that also happens to be what Qt does; buttons show their icons on 
> Linux (and other Unix variants?) but on OS X or MS Windows displaying of 
> those icons is deactivated unless you use a style that enables it. In fact, 
> the default setting for `SH_DialogButtonBox_ButtonsHaveIcons` is false except 
> in the generic Unix theme (= it does work globally like 
> `AA_DontShowIconsInMenus` everywhere else).
> 
> 2: a user who indicates s/he doesn't want to see icons will get an empty 
> button. That's also what can happen with QPushButton, and app developers have 
> to take this into consideration. Cf. toolbars (and Qt's position on the use 
> of "texted separators").
> I don't think I've ever come across a standard button showing only an 
> icon, except possibly the arrow button next to the progress indicators in 
> KMail and KDevelop.
> 
> As to fixing it here: as it turns out, "here" is the main source for 
> annoying icons rearing their silly heads on buttons on my screen ;) and it 
> was also something of a challenge to understand why they kept appearing 
> despite the fact that all code appeared to return the value of 
> `ShowIconsOnPushButtons`. Deprecated or not, it doesn't look like all 
> applications are going to stop using it anytime soon.
> 
> I looked into fixing the issue in KDialogButtonBox but saw that it does 
> nothing to override the `ShowIconsOnPushButtons` setting. The only way to 
> respect the setting through that class (or a modern equivalent) would be to 
> set an empty icon if `ShowIconsOnPushButtons=false`. That introduces another 
> regression: changes in this setting are supposed to be reflected by running 
> applications without requiring a restart (or a recreation of dialogs). If it 
> were just me I'd decree that buttons can have either text or an icon, but 
> right now we have to make do with this mixed situation.
> 
> I don't mind making this an OS X (and MS Windows) specific modification, 
> of course, but on those platforms
> 
> Thomas Lübking wrote:
> > 1: why should one care?
> 
> Because, as explained, that is what the hint says. Nothing else.
> 
> > I have found no sign in the code that the ShowIconsOnPushButtons hint 
> is to be used only for dialogs
> 
> No, but it's been used to feed SH_DialogButtonBox_ButtonsHaveIcons *AND* 
> KPushButton.
> ShowIconsOnPushButtons implied SH_DialogButtonBox_ButtonsHaveIcons but 
> NOT vv.
> 
> The approach is wrong, since you're abusing the hint for generalisation.
> 
> > but on OS X or MS Windows
> 
> ... Qt uses native elements which might simply globally downforce the 
> pushbutton icon nonsense (as could any style - I was more than once close to 
> doing that in virtuality)
> Eg. Breeze might do that on favor of the HIG, but that's not relevant 
> here.
> 
> Downforcing in KPushButton means to 

Review Request 126312: Add xcb variant for static KStartupInfo::sendFoo methods

[Martin Gräßlin]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126312/
---

Review request for KDE Frameworks, kwin and David Faure.


Repository: kwindowsystem


Description
---

Adds for each of sendStartup, sendChange, sendFinish an xcb variant
and deprecates the XLib variant. In addition the static variants which
are not windowing system specific delegate to the new xcb variant to
share more code paths.


Diffs
-

  src/kstartupinfo.cpp a97b8b5416ca67a083960a76093933fb098327a5 
  src/kstartupinfo.h dfcd42797d887ca6d43161f8c3b767ad436e5116 

Diff: https://git.reviewboard.kde.org/r/126312/diff/


Testing
---

autotest still passes


Thanks,

Martin Gräßlin

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126308: KPushButton: respect SH_DialogButtonBox_ButtonsHaveIcons, also when drawing

[Thomas Lübking]

> On Dec. 10, 2015, 10:11 p.m., Thomas Lübking wrote:
> > 1. What tells you that this is a dialog buttonbox pushbutton?
> > 2. What happens if the button has no text?
> > 
> > 
> > The bug is in QDialogButtonBox (or rather the K variant, 
> > QDialogButtonBoxPrivate::createButton() seems to incorporate the hint 
> > correctly)
> > 
> > 
> > [KDE] has "ShowIconsOnPushButtons=false" in kdeglobals which was 
> > interpreted by KPushButton _and_ kstyle for the hint, but the hint only 
> > covers Q'DialogButtonBox'es - there's simply no global rule for this like 
> > AA_DontShowIconsInMenus
> > 
> > => KDialogButtonBox shall respect the hint for its buttons (there're two 
> > special creation routines).
> > 
> > For the rest, the platform plugin ideally picks the kdeglobals setting and 
> > exports it to the application object (dyn property?) where the style can 
> > pick it and incorporate it into its calculations (ie. if no icons are 
> > wanted and there's text or arrow, omit the icon in size calculation and 
> > painting)
> > 
> > "Fixing" that in deprecated KPushButton doesn't really fix anything. We'll 
> > face the mix we had, just that users of QPushButton were far less prone to 
> > pass them an icon in pre-KF5 times.
> > 
> > Please also attach Hugo Pereira Da Costa.
> 
> René J.V. Bertin wrote:
> 1: why should one care? It is said nowhere that the setting defined as 
> "show icons on buttons" in `systemsettings(5)` applies only to dialogs. 
> Rather, the tooltip says "when this is enabled, KDE applications will show 
> icons alongside [sic!] some important buttons".
> In other words, when "this" is *not* enabled, there should be no icons, 
> period.
> I have found no sign in the code that the ShowIconsOnPushButtons hint is 
> to be used only for dialogs; `SH_DialogButtonBox_ButtonsHaveIcons` indeed 
> carries the suggestion in its name but I would not be surprised if Qt really 
> thinks of it in a more general sense. Probably also because the notion of 
> what a dialog is has become a lot vaguer.
> 
> And that also happens to be what Qt does; buttons show their icons on 
> Linux (and other Unix variants?) but on OS X or MS Windows displaying of 
> those icons is deactivated unless you use a style that enables it. In fact, 
> the default setting for `SH_DialogButtonBox_ButtonsHaveIcons` is false except 
> in the generic Unix theme (= it does work globally like 
> `AA_DontShowIconsInMenus` everywhere else).
> 
> 2: a user who indicates s/he doesn't want to see icons will get an empty 
> button. That's also what can happen with QPushButton, and app developers have 
> to take this into consideration. Cf. toolbars (and Qt's position on the use 
> of "texted separators").
> I don't think I've ever come across a standard button showing only an 
> icon, except possibly the arrow button next to the progress indicators in 
> KMail and KDevelop.
> 
> As to fixing it here: as it turns out, "here" is the main source for 
> annoying icons rearing their silly heads on buttons on my screen ;) and it 
> was also something of a challenge to understand why they kept appearing 
> despite the fact that all code appeared to return the value of 
> `ShowIconsOnPushButtons`. Deprecated or not, it doesn't look like all 
> applications are going to stop using it anytime soon.
> 
> I looked into fixing the issue in KDialogButtonBox but saw that it does 
> nothing to override the `ShowIconsOnPushButtons` setting. The only way to 
> respect the setting through that class (or a modern equivalent) would be to 
> set an empty icon if `ShowIconsOnPushButtons=false`. That introduces another 
> regression: changes in this setting are supposed to be reflected by running 
> applications without requiring a restart (or a recreation of dialogs). If it 
> were just me I'd decree that buttons can have either text or an icon, but 
> right now we have to make do with this mixed situation.
> 
> I don't mind making this an OS X (and MS Windows) specific modification, 
> of course, but on those platforms
> 
> Thomas Lübking wrote:
> > 1: why should one care?
> 
> Because, as explained, that is what the hint says. Nothing else.
> 
> > I have found no sign in the code that the ShowIconsOnPushButtons hint 
> is to be used only for dialogs
> 
> No, but it's been used to feed SH_DialogButtonBox_ButtonsHaveIcons *AND* 
> KPushButton.
> ShowIconsOnPushButtons implied SH_DialogButtonBox_ButtonsHaveIcons but 
> NOT vv.
> 
> The approach is wrong, since you're abusing the hint for generalisation.
> 
> > but on OS X or MS Windows
> 
> ... Qt uses native elements which might simply globally downforce the 
> pushbutton icon nonsense (as could any style - I was more than once close to 
> doing that in virtuality)
> Eg. Breeze might do that on favor of the HIG, but that's not relevant 
> here.
> 
> Downforcing in KPushButton means to 

Re: system tray test?

[Sebastian Kügler]

On Friday, December 11, 2015 12:19:51 PM René J.V. Bertin wrote:
> Does KF5 provide any classes above Qt's for putting up and controlling an
> icon (with/out menu) in the "system tray"? If so, is there a simple test
> app I can try?
> 
> Reason I'm asking: kwalletmanager5 isn't showing a systray interface like
> kwalletmanager(4) does. Not on OS X (where Qt5's systray does work), and on
> Linux under a KDE4 desktop I'm not sure what I'm getting.

Not sure what exactly you mean with "putting up and controlling an icon in the 
system tray", but we do have a status notifier test app, you can find it in: 
plasma-workspace/applets/systemtray/tests/statusnotifier

There are also some test programs in frameworks/knotifications/tests, you may 
want to look at.

Cheers,
-- 
sebas

http://www.kde.org | http://vizZzion.org

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126308: KPushButton: respect SH_DialogButtonBox_ButtonsHaveIcons, also when drawing

[Thomas Lübking]

> On Dec. 10, 2015, 10:11 p.m., Thomas Lübking wrote:
> > 1. What tells you that this is a dialog buttonbox pushbutton?
> > 2. What happens if the button has no text?
> > 
> > 
> > The bug is in QDialogButtonBox (or rather the K variant, 
> > QDialogButtonBoxPrivate::createButton() seems to incorporate the hint 
> > correctly)
> > 
> > 
> > [KDE] has "ShowIconsOnPushButtons=false" in kdeglobals which was 
> > interpreted by KPushButton _and_ kstyle for the hint, but the hint only 
> > covers Q'DialogButtonBox'es - there's simply no global rule for this like 
> > AA_DontShowIconsInMenus
> > 
> > => KDialogButtonBox shall respect the hint for its buttons (there're two 
> > special creation routines).
> > 
> > For the rest, the platform plugin ideally picks the kdeglobals setting and 
> > exports it to the application object (dyn property?) where the style can 
> > pick it and incorporate it into its calculations (ie. if no icons are 
> > wanted and there's text or arrow, omit the icon in size calculation and 
> > painting)
> > 
> > "Fixing" that in deprecated KPushButton doesn't really fix anything. We'll 
> > face the mix we had, just that users of QPushButton were far less prone to 
> > pass them an icon in pre-KF5 times.
> > 
> > Please also attach Hugo Pereira Da Costa.
> 
> René J.V. Bertin wrote:
> 1: why should one care? It is said nowhere that the setting defined as 
> "show icons on buttons" in `systemsettings(5)` applies only to dialogs. 
> Rather, the tooltip says "when this is enabled, KDE applications will show 
> icons alongside [sic!] some important buttons".
> In other words, when "this" is *not* enabled, there should be no icons, 
> period.
> I have found no sign in the code that the ShowIconsOnPushButtons hint is 
> to be used only for dialogs; `SH_DialogButtonBox_ButtonsHaveIcons` indeed 
> carries the suggestion in its name but I would not be surprised if Qt really 
> thinks of it in a more general sense. Probably also because the notion of 
> what a dialog is has become a lot vaguer.
> 
> And that also happens to be what Qt does; buttons show their icons on 
> Linux (and other Unix variants?) but on OS X or MS Windows displaying of 
> those icons is deactivated unless you use a style that enables it. In fact, 
> the default setting for `SH_DialogButtonBox_ButtonsHaveIcons` is false except 
> in the generic Unix theme (= it does work globally like 
> `AA_DontShowIconsInMenus` everywhere else).
> 
> 2: a user who indicates s/he doesn't want to see icons will get an empty 
> button. That's also what can happen with QPushButton, and app developers have 
> to take this into consideration. Cf. toolbars (and Qt's position on the use 
> of "texted separators").
> I don't think I've ever come across a standard button showing only an 
> icon, except possibly the arrow button next to the progress indicators in 
> KMail and KDevelop.
> 
> As to fixing it here: as it turns out, "here" is the main source for 
> annoying icons rearing their silly heads on buttons on my screen ;) and it 
> was also something of a challenge to understand why they kept appearing 
> despite the fact that all code appeared to return the value of 
> `ShowIconsOnPushButtons`. Deprecated or not, it doesn't look like all 
> applications are going to stop using it anytime soon.
> 
> I looked into fixing the issue in KDialogButtonBox but saw that it does 
> nothing to override the `ShowIconsOnPushButtons` setting. The only way to 
> respect the setting through that class (or a modern equivalent) would be to 
> set an empty icon if `ShowIconsOnPushButtons=false`. That introduces another 
> regression: changes in this setting are supposed to be reflected by running 
> applications without requiring a restart (or a recreation of dialogs). If it 
> were just me I'd decree that buttons can have either text or an icon, but 
> right now we have to make do with this mixed situation.
> 
> I don't mind making this an OS X (and MS Windows) specific modification, 
> of course, but on those platforms
> 
> Thomas Lübking wrote:
> > 1: why should one care?
> 
> Because, as explained, that is what the hint says. Nothing else.
> 
> > I have found no sign in the code that the ShowIconsOnPushButtons hint 
> is to be used only for dialogs
> 
> No, but it's been used to feed SH_DialogButtonBox_ButtonsHaveIcons *AND* 
> KPushButton.
> ShowIconsOnPushButtons implied SH_DialogButtonBox_ButtonsHaveIcons but 
> NOT vv.
> 
> The approach is wrong, since you're abusing the hint for generalisation.
> 
> > but on OS X or MS Windows
> 
> ... Qt uses native elements which might simply globally downforce the 
> pushbutton icon nonsense (as could any style - I was more than once close to 
> doing that in virtuality)
> Eg. Breeze might do that on favor of the HIG, but that's not relevant 
> here.
> 
> Downforcing in KPushButton means to 

Re: Review Request 126308: KPushButton: respect SH_DialogButtonBox_ButtonsHaveIcons, also when drawing

[Hugo Pereira Da Costa]

> On Dec. 10, 2015, 10:11 p.m., Thomas Lübking wrote:
> > 1. What tells you that this is a dialog buttonbox pushbutton?
> > 2. What happens if the button has no text?
> > 
> > 
> > The bug is in QDialogButtonBox (or rather the K variant, 
> > QDialogButtonBoxPrivate::createButton() seems to incorporate the hint 
> > correctly)
> > 
> > 
> > [KDE] has "ShowIconsOnPushButtons=false" in kdeglobals which was 
> > interpreted by KPushButton _and_ kstyle for the hint, but the hint only 
> > covers Q'DialogButtonBox'es - there's simply no global rule for this like 
> > AA_DontShowIconsInMenus
> > 
> > => KDialogButtonBox shall respect the hint for its buttons (there're two 
> > special creation routines).
> > 
> > For the rest, the platform plugin ideally picks the kdeglobals setting and 
> > exports it to the application object (dyn property?) where the style can 
> > pick it and incorporate it into its calculations (ie. if no icons are 
> > wanted and there's text or arrow, omit the icon in size calculation and 
> > painting)
> > 
> > "Fixing" that in deprecated KPushButton doesn't really fix anything. We'll 
> > face the mix we had, just that users of QPushButton were far less prone to 
> > pass them an icon in pre-KF5 times.
> > 
> > Please also attach Hugo Pereira Da Costa.
> 
> René J.V. Bertin wrote:
> 1: why should one care? It is said nowhere that the setting defined as 
> "show icons on buttons" in `systemsettings(5)` applies only to dialogs. 
> Rather, the tooltip says "when this is enabled, KDE applications will show 
> icons alongside [sic!] some important buttons".
> In other words, when "this" is *not* enabled, there should be no icons, 
> period.
> I have found no sign in the code that the ShowIconsOnPushButtons hint is 
> to be used only for dialogs; `SH_DialogButtonBox_ButtonsHaveIcons` indeed 
> carries the suggestion in its name but I would not be surprised if Qt really 
> thinks of it in a more general sense. Probably also because the notion of 
> what a dialog is has become a lot vaguer.
> 
> And that also happens to be what Qt does; buttons show their icons on 
> Linux (and other Unix variants?) but on OS X or MS Windows displaying of 
> those icons is deactivated unless you use a style that enables it. In fact, 
> the default setting for `SH_DialogButtonBox_ButtonsHaveIcons` is false except 
> in the generic Unix theme (= it does work globally like 
> `AA_DontShowIconsInMenus` everywhere else).
> 
> 2: a user who indicates s/he doesn't want to see icons will get an empty 
> button. That's also what can happen with QPushButton, and app developers have 
> to take this into consideration. Cf. toolbars (and Qt's position on the use 
> of "texted separators").
> I don't think I've ever come across a standard button showing only an 
> icon, except possibly the arrow button next to the progress indicators in 
> KMail and KDevelop.
> 
> As to fixing it here: as it turns out, "here" is the main source for 
> annoying icons rearing their silly heads on buttons on my screen ;) and it 
> was also something of a challenge to understand why they kept appearing 
> despite the fact that all code appeared to return the value of 
> `ShowIconsOnPushButtons`. Deprecated or not, it doesn't look like all 
> applications are going to stop using it anytime soon.
> 
> I looked into fixing the issue in KDialogButtonBox but saw that it does 
> nothing to override the `ShowIconsOnPushButtons` setting. The only way to 
> respect the setting through that class (or a modern equivalent) would be to 
> set an empty icon if `ShowIconsOnPushButtons=false`. That introduces another 
> regression: changes in this setting are supposed to be reflected by running 
> applications without requiring a restart (or a recreation of dialogs). If it 
> were just me I'd decree that buttons can have either text or an icon, but 
> right now we have to make do with this mixed situation.
> 
> I don't mind making this an OS X (and MS Windows) specific modification, 
> of course, but on those platforms
> 
> Thomas Lübking wrote:
> > 1: why should one care?
> 
> Because, as explained, that is what the hint says. Nothing else.
> 
> > I have found no sign in the code that the ShowIconsOnPushButtons hint 
> is to be used only for dialogs
> 
> No, but it's been used to feed SH_DialogButtonBox_ButtonsHaveIcons *AND* 
> KPushButton.
> ShowIconsOnPushButtons implied SH_DialogButtonBox_ButtonsHaveIcons but 
> NOT vv.
> 
> The approach is wrong, since you're abusing the hint for generalisation.
> 
> > but on OS X or MS Windows
> 
> ... Qt uses native elements which might simply globally downforce the 
> pushbutton icon nonsense (as could any style - I was more than once close to 
> doing that in virtuality)
> Eg. Breeze might do that on favor of the HIG, but that's not relevant 
> here.
> 
> Downforcing in KPushButton means to 

Review Request 126313: Use an xcb for interaction with KStartupInfo

[Martin Gräßlin]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126313/
---

Review request for KDE Frameworks and David Faure.


Repository: kinit


Description
---

By changing to xcb we can use NETRootInfo to get the current desktop
and don't need to duplicate the logic. Also it means that more code
is ported from XLib to xcb and might allow us to drop the XLib
dependency in future.


Diffs
-

  src/kdeinit/kinit.cpp a18008a11bf00a35aa0cab450180926217cd58f5 
  src/kdeinit/CMakeLists.txt f94db717f2403b602648286eac243837d8714069 
  src/config-kdeinit.h.cmake cfcfc61a1bb560ff9a902b89bd3b8fb27273ebf2 
  CMakeLists.txt eeecab775cfb5dfe12cf9c4991c658cc261ed727 

Diff: https://git.reviewboard.kde.org/r/126313/diff/


Testing
---


Thanks,

Martin Gräßlin

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126308: KPushButton: respect SH_DialogButtonBox_ButtonsHaveIcons, also when drawing

[René J . V . Bertin]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126308/
---

(Updated Dec. 11, 2015, 1:42 p.m.)


Review request for KDE Software on Mac OS X, KDE Frameworks, Qt KDE, and Hugo 
Pereira Da Costa.


Changes
---

This adds the changes to KDialogButtonBox that seem required to respect 
`SH_DialogButtonBox_ButtonsHaveIcons` regardless of `ShowIconsOnPushButtons` (= 
if the former could be independent of the latter e.g. when using a style that 
does not use the latter to determine the value of the former).

What is the point in allowing `KDialogButtonBox::addButton` to create a button 
that is not added because of an invalid role? It seems that button wouldn't 
appear (or in an unexpected place), and be leaked?


Repository: kdelibs4support


Description
---

KF5 applications have long had a habit of drawing icons on buttons even when 
this feature was turned off in the user's setting. This was mostly noticeable 
in applications built on kdelibs4support.

It seems that the actual culprit is in Qt's QPushButton implementation 
(https://bugreports.qt.io/browse/QTBUG-49887), but it is possible to work 
around it in `KPushButton::paintEvent`, by removing the icon (forcing it to the 
null icon) in the option instance, before handing off control to the painter.


Diffs (updated)
-

  src/kdeui/kpushbutton.cpp 98534fa 
  src/kdeui/kdialogbuttonbox.cpp 0f6649b 

Diff: https://git.reviewboard.kde.org/r/126308/diff/


Testing
---

On Kubuntu 14.04 and OS X 10.9.5 with Qt 5.5.1 and KF5 frameworks 5.16.0 .

I have not yet verified if there are other classes where this modification 
would be relevant too.


Thanks,

René J.V. Bertin

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126308: KPushButton: respect SH_DialogButtonBox_ButtonsHaveIcons, also when drawing

[Hugo Pereira Da Costa]

> On Dec. 10, 2015, 10:11 p.m., Thomas Lübking wrote:
> > 1. What tells you that this is a dialog buttonbox pushbutton?
> > 2. What happens if the button has no text?
> > 
> > 
> > The bug is in QDialogButtonBox (or rather the K variant, 
> > QDialogButtonBoxPrivate::createButton() seems to incorporate the hint 
> > correctly)
> > 
> > 
> > [KDE] has "ShowIconsOnPushButtons=false" in kdeglobals which was 
> > interpreted by KPushButton _and_ kstyle for the hint, but the hint only 
> > covers Q'DialogButtonBox'es - there's simply no global rule for this like 
> > AA_DontShowIconsInMenus
> > 
> > => KDialogButtonBox shall respect the hint for its buttons (there're two 
> > special creation routines).
> > 
> > For the rest, the platform plugin ideally picks the kdeglobals setting and 
> > exports it to the application object (dyn property?) where the style can 
> > pick it and incorporate it into its calculations (ie. if no icons are 
> > wanted and there's text or arrow, omit the icon in size calculation and 
> > painting)
> > 
> > "Fixing" that in deprecated KPushButton doesn't really fix anything. We'll 
> > face the mix we had, just that users of QPushButton were far less prone to 
> > pass them an icon in pre-KF5 times.
> > 
> > Please also attach Hugo Pereira Da Costa.
> 
> René J.V. Bertin wrote:
> 1: why should one care? It is said nowhere that the setting defined as 
> "show icons on buttons" in `systemsettings(5)` applies only to dialogs. 
> Rather, the tooltip says "when this is enabled, KDE applications will show 
> icons alongside [sic!] some important buttons".
> In other words, when "this" is *not* enabled, there should be no icons, 
> period.
> I have found no sign in the code that the ShowIconsOnPushButtons hint is 
> to be used only for dialogs; `SH_DialogButtonBox_ButtonsHaveIcons` indeed 
> carries the suggestion in its name but I would not be surprised if Qt really 
> thinks of it in a more general sense. Probably also because the notion of 
> what a dialog is has become a lot vaguer.
> 
> And that also happens to be what Qt does; buttons show their icons on 
> Linux (and other Unix variants?) but on OS X or MS Windows displaying of 
> those icons is deactivated unless you use a style that enables it. In fact, 
> the default setting for `SH_DialogButtonBox_ButtonsHaveIcons` is false except 
> in the generic Unix theme (= it does work globally like 
> `AA_DontShowIconsInMenus` everywhere else).
> 
> 2: a user who indicates s/he doesn't want to see icons will get an empty 
> button. That's also what can happen with QPushButton, and app developers have 
> to take this into consideration. Cf. toolbars (and Qt's position on the use 
> of "texted separators").
> I don't think I've ever come across a standard button showing only an 
> icon, except possibly the arrow button next to the progress indicators in 
> KMail and KDevelop.
> 
> As to fixing it here: as it turns out, "here" is the main source for 
> annoying icons rearing their silly heads on buttons on my screen ;) and it 
> was also something of a challenge to understand why they kept appearing 
> despite the fact that all code appeared to return the value of 
> `ShowIconsOnPushButtons`. Deprecated or not, it doesn't look like all 
> applications are going to stop using it anytime soon.
> 
> I looked into fixing the issue in KDialogButtonBox but saw that it does 
> nothing to override the `ShowIconsOnPushButtons` setting. The only way to 
> respect the setting through that class (or a modern equivalent) would be to 
> set an empty icon if `ShowIconsOnPushButtons=false`. That introduces another 
> regression: changes in this setting are supposed to be reflected by running 
> applications without requiring a restart (or a recreation of dialogs). If it 
> were just me I'd decree that buttons can have either text or an icon, but 
> right now we have to make do with this mixed situation.
> 
> I don't mind making this an OS X (and MS Windows) specific modification, 
> of course, but on those platforms
> 
> Thomas Lübking wrote:
> > 1: why should one care?
> 
> Because, as explained, that is what the hint says. Nothing else.
> 
> > I have found no sign in the code that the ShowIconsOnPushButtons hint 
> is to be used only for dialogs
> 
> No, but it's been used to feed SH_DialogButtonBox_ButtonsHaveIcons *AND* 
> KPushButton.
> ShowIconsOnPushButtons implied SH_DialogButtonBox_ButtonsHaveIcons but 
> NOT vv.
> 
> The approach is wrong, since you're abusing the hint for generalisation.
> 
> > but on OS X or MS Windows
> 
> ... Qt uses native elements which might simply globally downforce the 
> pushbutton icon nonsense (as could any style - I was more than once close to 
> doing that in virtuality)
> Eg. Breeze might do that on favor of the HIG, but that's not relevant 
> here.
> 
> Downforcing in KPushButton means to 

Re: Review Request 126308: KPushButton: respect SH_DialogButtonBox_ButtonsHaveIcons, also when drawing

[René J . V . Bertin]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126308/
---

(Updated Dec. 11, 2015, 1:59 p.m.)


Review request for KDE Software on Mac OS X, KDE Frameworks, Qt KDE, and Hugo 
Pereira Da Costa.


Changes
---

This patch for KDialogButtonBox actually builds.


Repository: kdelibs4support


Description
---

KF5 applications have long had a habit of drawing icons on buttons even when 
this feature was turned off in the user's setting. This was mostly noticeable 
in applications built on kdelibs4support.

It seems that the actual culprit is in Qt's QPushButton implementation 
(https://bugreports.qt.io/browse/QTBUG-49887), but it is possible to work 
around it in `KPushButton::paintEvent`, by removing the icon (forcing it to the 
null icon) in the option instance, before handing off control to the painter.


Diffs (updated)
-

  src/kdeui/kdialogbuttonbox.cpp 0f6649b 
  src/kdeui/kpushbutton.cpp 98534fa 

Diff: https://git.reviewboard.kde.org/r/126308/diff/


Testing
---

On Kubuntu 14.04 and OS X 10.9.5 with Qt 5.5.1 and KF5 frameworks 5.16.0 .

I have not yet verified if there are other classes where this modification 
would be relevant too.


Thanks,

René J.V. Bertin

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126246: Add test for dynamically changing file definitions

[Sebastian Kügler]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126246/#review89354
---

Ship it!


Ship It!


autotests/packagestructuretest.cpp (line 102)


ws (just because RB makes it totally red)



autotests/packagestructuretest.cpp (line 108)


seems unnecessary?


- Sebastian Kügler


On Dec. 4, 2015, 6:23 p.m., Marco Martin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126246/
> ---
> 
> (Updated Dec. 4, 2015, 6:23 p.m.)
> 
> 
> Review request for KDE Frameworks and Plasma.
> 
> 
> Repository: kpackage
> 
> 
> Description
> ---
> 
> this, referred to https://git.reviewboard.kde.org/r/126244/ tests that adding 
> or removing a file definition depending on the path (and whatever criteria, 
> like metadata contents) works. since is already done in several places has to 
> work correctly
> 
> 
> Diffs
> -
> 
>   autotests/packagestructuretest.h de2038e 
>   autotests/packagestructuretest.cpp 4784bfd 
> 
> Diff: https://git.reviewboard.kde.org/r/126246/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Marco Martin
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126308: KPushButton: respect SH_DialogButtonBox_ButtonsHaveIcons, also when drawing

[Thomas Lübking]

> On Dec. 10, 2015, 10:11 p.m., Thomas Lübking wrote:
> > 1. What tells you that this is a dialog buttonbox pushbutton?
> > 2. What happens if the button has no text?
> > 
> > 
> > The bug is in QDialogButtonBox (or rather the K variant, 
> > QDialogButtonBoxPrivate::createButton() seems to incorporate the hint 
> > correctly)
> > 
> > 
> > [KDE] has "ShowIconsOnPushButtons=false" in kdeglobals which was 
> > interpreted by KPushButton _and_ kstyle for the hint, but the hint only 
> > covers Q'DialogButtonBox'es - there's simply no global rule for this like 
> > AA_DontShowIconsInMenus
> > 
> > => KDialogButtonBox shall respect the hint for its buttons (there're two 
> > special creation routines).
> > 
> > For the rest, the platform plugin ideally picks the kdeglobals setting and 
> > exports it to the application object (dyn property?) where the style can 
> > pick it and incorporate it into its calculations (ie. if no icons are 
> > wanted and there's text or arrow, omit the icon in size calculation and 
> > painting)
> > 
> > "Fixing" that in deprecated KPushButton doesn't really fix anything. We'll 
> > face the mix we had, just that users of QPushButton were far less prone to 
> > pass them an icon in pre-KF5 times.
> > 
> > Please also attach Hugo Pereira Da Costa.
> 
> René J.V. Bertin wrote:
> 1: why should one care? It is said nowhere that the setting defined as 
> "show icons on buttons" in `systemsettings(5)` applies only to dialogs. 
> Rather, the tooltip says "when this is enabled, KDE applications will show 
> icons alongside [sic!] some important buttons".
> In other words, when "this" is *not* enabled, there should be no icons, 
> period.
> I have found no sign in the code that the ShowIconsOnPushButtons hint is 
> to be used only for dialogs; `SH_DialogButtonBox_ButtonsHaveIcons` indeed 
> carries the suggestion in its name but I would not be surprised if Qt really 
> thinks of it in a more general sense. Probably also because the notion of 
> what a dialog is has become a lot vaguer.
> 
> And that also happens to be what Qt does; buttons show their icons on 
> Linux (and other Unix variants?) but on OS X or MS Windows displaying of 
> those icons is deactivated unless you use a style that enables it. In fact, 
> the default setting for `SH_DialogButtonBox_ButtonsHaveIcons` is false except 
> in the generic Unix theme (= it does work globally like 
> `AA_DontShowIconsInMenus` everywhere else).
> 
> 2: a user who indicates s/he doesn't want to see icons will get an empty 
> button. That's also what can happen with QPushButton, and app developers have 
> to take this into consideration. Cf. toolbars (and Qt's position on the use 
> of "texted separators").
> I don't think I've ever come across a standard button showing only an 
> icon, except possibly the arrow button next to the progress indicators in 
> KMail and KDevelop.
> 
> As to fixing it here: as it turns out, "here" is the main source for 
> annoying icons rearing their silly heads on buttons on my screen ;) and it 
> was also something of a challenge to understand why they kept appearing 
> despite the fact that all code appeared to return the value of 
> `ShowIconsOnPushButtons`. Deprecated or not, it doesn't look like all 
> applications are going to stop using it anytime soon.
> 
> I looked into fixing the issue in KDialogButtonBox but saw that it does 
> nothing to override the `ShowIconsOnPushButtons` setting. The only way to 
> respect the setting through that class (or a modern equivalent) would be to 
> set an empty icon if `ShowIconsOnPushButtons=false`. That introduces another 
> regression: changes in this setting are supposed to be reflected by running 
> applications without requiring a restart (or a recreation of dialogs). If it 
> were just me I'd decree that buttons can have either text or an icon, but 
> right now we have to make do with this mixed situation.
> 
> I don't mind making this an OS X (and MS Windows) specific modification, 
> of course, but on those platforms
> 
> Thomas Lübking wrote:
> > 1: why should one care?
> 
> Because, as explained, that is what the hint says. Nothing else.
> 
> > I have found no sign in the code that the ShowIconsOnPushButtons hint 
> is to be used only for dialogs
> 
> No, but it's been used to feed SH_DialogButtonBox_ButtonsHaveIcons *AND* 
> KPushButton.
> ShowIconsOnPushButtons implied SH_DialogButtonBox_ButtonsHaveIcons but 
> NOT vv.
> 
> The approach is wrong, since you're abusing the hint for generalisation.
> 
> > but on OS X or MS Windows
> 
> ... Qt uses native elements which might simply globally downforce the 
> pushbutton icon nonsense (as could any style - I was more than once close to 
> doing that in virtuality)
> Eg. Breeze might do that on favor of the HIG, but that's not relevant 
> here.
> 
> Downforcing in KPushButton means to 

Re: Review Request 126308: KPushButton: respect SH_DialogButtonBox_ButtonsHaveIcons, also when drawing

[René J . V . Bertin]

> On Dec. 10, 2015, 11:11 p.m., Thomas Lübking wrote:
> > 1. What tells you that this is a dialog buttonbox pushbutton?
> > 2. What happens if the button has no text?
> > 
> > 
> > The bug is in QDialogButtonBox (or rather the K variant, 
> > QDialogButtonBoxPrivate::createButton() seems to incorporate the hint 
> > correctly)
> > 
> > 
> > [KDE] has "ShowIconsOnPushButtons=false" in kdeglobals which was 
> > interpreted by KPushButton _and_ kstyle for the hint, but the hint only 
> > covers Q'DialogButtonBox'es - there's simply no global rule for this like 
> > AA_DontShowIconsInMenus
> > 
> > => KDialogButtonBox shall respect the hint for its buttons (there're two 
> > special creation routines).
> > 
> > For the rest, the platform plugin ideally picks the kdeglobals setting and 
> > exports it to the application object (dyn property?) where the style can 
> > pick it and incorporate it into its calculations (ie. if no icons are 
> > wanted and there's text or arrow, omit the icon in size calculation and 
> > painting)
> > 
> > "Fixing" that in deprecated KPushButton doesn't really fix anything. We'll 
> > face the mix we had, just that users of QPushButton were far less prone to 
> > pass them an icon in pre-KF5 times.
> > 
> > Please also attach Hugo Pereira Da Costa.
> 
> René J.V. Bertin wrote:
> 1: why should one care? It is said nowhere that the setting defined as 
> "show icons on buttons" in `systemsettings(5)` applies only to dialogs. 
> Rather, the tooltip says "when this is enabled, KDE applications will show 
> icons alongside [sic!] some important buttons".
> In other words, when "this" is *not* enabled, there should be no icons, 
> period.
> I have found no sign in the code that the ShowIconsOnPushButtons hint is 
> to be used only for dialogs; `SH_DialogButtonBox_ButtonsHaveIcons` indeed 
> carries the suggestion in its name but I would not be surprised if Qt really 
> thinks of it in a more general sense. Probably also because the notion of 
> what a dialog is has become a lot vaguer.
> 
> And that also happens to be what Qt does; buttons show their icons on 
> Linux (and other Unix variants?) but on OS X or MS Windows displaying of 
> those icons is deactivated unless you use a style that enables it. In fact, 
> the default setting for `SH_DialogButtonBox_ButtonsHaveIcons` is false except 
> in the generic Unix theme (= it does work globally like 
> `AA_DontShowIconsInMenus` everywhere else).
> 
> 2: a user who indicates s/he doesn't want to see icons will get an empty 
> button. That's also what can happen with QPushButton, and app developers have 
> to take this into consideration. Cf. toolbars (and Qt's position on the use 
> of "texted separators").
> I don't think I've ever come across a standard button showing only an 
> icon, except possibly the arrow button next to the progress indicators in 
> KMail and KDevelop.
> 
> As to fixing it here: as it turns out, "here" is the main source for 
> annoying icons rearing their silly heads on buttons on my screen ;) and it 
> was also something of a challenge to understand why they kept appearing 
> despite the fact that all code appeared to return the value of 
> `ShowIconsOnPushButtons`. Deprecated or not, it doesn't look like all 
> applications are going to stop using it anytime soon.
> 
> I looked into fixing the issue in KDialogButtonBox but saw that it does 
> nothing to override the `ShowIconsOnPushButtons` setting. The only way to 
> respect the setting through that class (or a modern equivalent) would be to 
> set an empty icon if `ShowIconsOnPushButtons=false`. That introduces another 
> regression: changes in this setting are supposed to be reflected by running 
> applications without requiring a restart (or a recreation of dialogs). If it 
> were just me I'd decree that buttons can have either text or an icon, but 
> right now we have to make do with this mixed situation.
> 
> I don't mind making this an OS X (and MS Windows) specific modification, 
> of course, but on those platforms
> 
> Thomas Lübking wrote:
> > 1: why should one care?
> 
> Because, as explained, that is what the hint says. Nothing else.
> 
> > I have found no sign in the code that the ShowIconsOnPushButtons hint 
> is to be used only for dialogs
> 
> No, but it's been used to feed SH_DialogButtonBox_ButtonsHaveIcons *AND* 
> KPushButton.
> ShowIconsOnPushButtons implied SH_DialogButtonBox_ButtonsHaveIcons but 
> NOT vv.
> 
> The approach is wrong, since you're abusing the hint for generalisation.
> 
> > but on OS X or MS Windows
> 
> ... Qt uses native elements which might simply globally downforce the 
> pushbutton icon nonsense (as could any style - I was more than once close to 
> doing that in virtuality)
> Eg. Breeze might do that on favor of the HIG, but that's not relevant 
> here.
> 
> Downforcing in KPushButton means to 

Re: Review Request 126308: KPushButton: respect SH_DialogButtonBox_ButtonsHaveIcons, also when drawing

[Thomas Lübking]

> On Dec. 10, 2015, 10:11 p.m., Thomas Lübking wrote:
> > 1. What tells you that this is a dialog buttonbox pushbutton?
> > 2. What happens if the button has no text?
> > 
> > 
> > The bug is in QDialogButtonBox (or rather the K variant, 
> > QDialogButtonBoxPrivate::createButton() seems to incorporate the hint 
> > correctly)
> > 
> > 
> > [KDE] has "ShowIconsOnPushButtons=false" in kdeglobals which was 
> > interpreted by KPushButton _and_ kstyle for the hint, but the hint only 
> > covers Q'DialogButtonBox'es - there's simply no global rule for this like 
> > AA_DontShowIconsInMenus
> > 
> > => KDialogButtonBox shall respect the hint for its buttons (there're two 
> > special creation routines).
> > 
> > For the rest, the platform plugin ideally picks the kdeglobals setting and 
> > exports it to the application object (dyn property?) where the style can 
> > pick it and incorporate it into its calculations (ie. if no icons are 
> > wanted and there's text or arrow, omit the icon in size calculation and 
> > painting)
> > 
> > "Fixing" that in deprecated KPushButton doesn't really fix anything. We'll 
> > face the mix we had, just that users of QPushButton were far less prone to 
> > pass them an icon in pre-KF5 times.
> > 
> > Please also attach Hugo Pereira Da Costa.
> 
> René J.V. Bertin wrote:
> 1: why should one care? It is said nowhere that the setting defined as 
> "show icons on buttons" in `systemsettings(5)` applies only to dialogs. 
> Rather, the tooltip says "when this is enabled, KDE applications will show 
> icons alongside [sic!] some important buttons".
> In other words, when "this" is *not* enabled, there should be no icons, 
> period.
> I have found no sign in the code that the ShowIconsOnPushButtons hint is 
> to be used only for dialogs; `SH_DialogButtonBox_ButtonsHaveIcons` indeed 
> carries the suggestion in its name but I would not be surprised if Qt really 
> thinks of it in a more general sense. Probably also because the notion of 
> what a dialog is has become a lot vaguer.
> 
> And that also happens to be what Qt does; buttons show their icons on 
> Linux (and other Unix variants?) but on OS X or MS Windows displaying of 
> those icons is deactivated unless you use a style that enables it. In fact, 
> the default setting for `SH_DialogButtonBox_ButtonsHaveIcons` is false except 
> in the generic Unix theme (= it does work globally like 
> `AA_DontShowIconsInMenus` everywhere else).
> 
> 2: a user who indicates s/he doesn't want to see icons will get an empty 
> button. That's also what can happen with QPushButton, and app developers have 
> to take this into consideration. Cf. toolbars (and Qt's position on the use 
> of "texted separators").
> I don't think I've ever come across a standard button showing only an 
> icon, except possibly the arrow button next to the progress indicators in 
> KMail and KDevelop.
> 
> As to fixing it here: as it turns out, "here" is the main source for 
> annoying icons rearing their silly heads on buttons on my screen ;) and it 
> was also something of a challenge to understand why they kept appearing 
> despite the fact that all code appeared to return the value of 
> `ShowIconsOnPushButtons`. Deprecated or not, it doesn't look like all 
> applications are going to stop using it anytime soon.
> 
> I looked into fixing the issue in KDialogButtonBox but saw that it does 
> nothing to override the `ShowIconsOnPushButtons` setting. The only way to 
> respect the setting through that class (or a modern equivalent) would be to 
> set an empty icon if `ShowIconsOnPushButtons=false`. That introduces another 
> regression: changes in this setting are supposed to be reflected by running 
> applications without requiring a restart (or a recreation of dialogs). If it 
> were just me I'd decree that buttons can have either text or an icon, but 
> right now we have to make do with this mixed situation.
> 
> I don't mind making this an OS X (and MS Windows) specific modification, 
> of course, but on those platforms
> 
> Thomas Lübking wrote:
> > 1: why should one care?
> 
> Because, as explained, that is what the hint says. Nothing else.
> 
> > I have found no sign in the code that the ShowIconsOnPushButtons hint 
> is to be used only for dialogs
> 
> No, but it's been used to feed SH_DialogButtonBox_ButtonsHaveIcons *AND* 
> KPushButton.
> ShowIconsOnPushButtons implied SH_DialogButtonBox_ButtonsHaveIcons but 
> NOT vv.
> 
> The approach is wrong, since you're abusing the hint for generalisation.
> 
> > but on OS X or MS Windows
> 
> ... Qt uses native elements which might simply globally downforce the 
> pushbutton icon nonsense (as could any style - I was more than once close to 
> doing that in virtuality)
> Eg. Breeze might do that on favor of the HIG, but that's not relevant 
> here.
> 
> Downforcing in KPushButton means to 

Re: Review Request 126308: KPushButton: respect SH_DialogButtonBox_ButtonsHaveIcons, also when drawing

[René J . V . Bertin]

> On Dec. 10, 2015, 11:11 p.m., Thomas Lübking wrote:
> > 1. What tells you that this is a dialog buttonbox pushbutton?
> > 2. What happens if the button has no text?
> > 
> > 
> > The bug is in QDialogButtonBox (or rather the K variant, 
> > QDialogButtonBoxPrivate::createButton() seems to incorporate the hint 
> > correctly)
> > 
> > 
> > [KDE] has "ShowIconsOnPushButtons=false" in kdeglobals which was 
> > interpreted by KPushButton _and_ kstyle for the hint, but the hint only 
> > covers Q'DialogButtonBox'es - there's simply no global rule for this like 
> > AA_DontShowIconsInMenus
> > 
> > => KDialogButtonBox shall respect the hint for its buttons (there're two 
> > special creation routines).
> > 
> > For the rest, the platform plugin ideally picks the kdeglobals setting and 
> > exports it to the application object (dyn property?) where the style can 
> > pick it and incorporate it into its calculations (ie. if no icons are 
> > wanted and there's text or arrow, omit the icon in size calculation and 
> > painting)
> > 
> > "Fixing" that in deprecated KPushButton doesn't really fix anything. We'll 
> > face the mix we had, just that users of QPushButton were far less prone to 
> > pass them an icon in pre-KF5 times.
> > 
> > Please also attach Hugo Pereira Da Costa.
> 
> René J.V. Bertin wrote:
> 1: why should one care? It is said nowhere that the setting defined as 
> "show icons on buttons" in `systemsettings(5)` applies only to dialogs. 
> Rather, the tooltip says "when this is enabled, KDE applications will show 
> icons alongside [sic!] some important buttons".
> In other words, when "this" is *not* enabled, there should be no icons, 
> period.
> I have found no sign in the code that the ShowIconsOnPushButtons hint is 
> to be used only for dialogs; `SH_DialogButtonBox_ButtonsHaveIcons` indeed 
> carries the suggestion in its name but I would not be surprised if Qt really 
> thinks of it in a more general sense. Probably also because the notion of 
> what a dialog is has become a lot vaguer.
> 
> And that also happens to be what Qt does; buttons show their icons on 
> Linux (and other Unix variants?) but on OS X or MS Windows displaying of 
> those icons is deactivated unless you use a style that enables it. In fact, 
> the default setting for `SH_DialogButtonBox_ButtonsHaveIcons` is false except 
> in the generic Unix theme (= it does work globally like 
> `AA_DontShowIconsInMenus` everywhere else).
> 
> 2: a user who indicates s/he doesn't want to see icons will get an empty 
> button. That's also what can happen with QPushButton, and app developers have 
> to take this into consideration. Cf. toolbars (and Qt's position on the use 
> of "texted separators").
> I don't think I've ever come across a standard button showing only an 
> icon, except possibly the arrow button next to the progress indicators in 
> KMail and KDevelop.
> 
> As to fixing it here: as it turns out, "here" is the main source for 
> annoying icons rearing their silly heads on buttons on my screen ;) and it 
> was also something of a challenge to understand why they kept appearing 
> despite the fact that all code appeared to return the value of 
> `ShowIconsOnPushButtons`. Deprecated or not, it doesn't look like all 
> applications are going to stop using it anytime soon.
> 
> I looked into fixing the issue in KDialogButtonBox but saw that it does 
> nothing to override the `ShowIconsOnPushButtons` setting. The only way to 
> respect the setting through that class (or a modern equivalent) would be to 
> set an empty icon if `ShowIconsOnPushButtons=false`. That introduces another 
> regression: changes in this setting are supposed to be reflected by running 
> applications without requiring a restart (or a recreation of dialogs). If it 
> were just me I'd decree that buttons can have either text or an icon, but 
> right now we have to make do with this mixed situation.
> 
> I don't mind making this an OS X (and MS Windows) specific modification, 
> of course, but on those platforms
> 
> Thomas Lübking wrote:
> > 1: why should one care?
> 
> Because, as explained, that is what the hint says. Nothing else.
> 
> > I have found no sign in the code that the ShowIconsOnPushButtons hint 
> is to be used only for dialogs
> 
> No, but it's been used to feed SH_DialogButtonBox_ButtonsHaveIcons *AND* 
> KPushButton.
> ShowIconsOnPushButtons implied SH_DialogButtonBox_ButtonsHaveIcons but 
> NOT vv.
> 
> The approach is wrong, since you're abusing the hint for generalisation.
> 
> > but on OS X or MS Windows
> 
> ... Qt uses native elements which might simply globally downforce the 
> pushbutton icon nonsense (as could any style - I was more than once close to 
> doing that in virtuality)
> Eg. Breeze might do that on favor of the HIG, but that's not relevant 
> here.
> 
> Downforcing in KPushButton means to 

Re: Review Request 126308: KPushButton: respect SH_DialogButtonBox_ButtonsHaveIcons, also when drawing

[René J . V . Bertin]

> On Dec. 10, 2015, 11:11 p.m., Thomas Lübking wrote:
> > 1. What tells you that this is a dialog buttonbox pushbutton?
> > 2. What happens if the button has no text?
> > 
> > 
> > The bug is in QDialogButtonBox (or rather the K variant, 
> > QDialogButtonBoxPrivate::createButton() seems to incorporate the hint 
> > correctly)
> > 
> > 
> > [KDE] has "ShowIconsOnPushButtons=false" in kdeglobals which was 
> > interpreted by KPushButton _and_ kstyle for the hint, but the hint only 
> > covers Q'DialogButtonBox'es - there's simply no global rule for this like 
> > AA_DontShowIconsInMenus
> > 
> > => KDialogButtonBox shall respect the hint for its buttons (there're two 
> > special creation routines).
> > 
> > For the rest, the platform plugin ideally picks the kdeglobals setting and 
> > exports it to the application object (dyn property?) where the style can 
> > pick it and incorporate it into its calculations (ie. if no icons are 
> > wanted and there's text or arrow, omit the icon in size calculation and 
> > painting)
> > 
> > "Fixing" that in deprecated KPushButton doesn't really fix anything. We'll 
> > face the mix we had, just that users of QPushButton were far less prone to 
> > pass them an icon in pre-KF5 times.
> > 
> > Please also attach Hugo Pereira Da Costa.
> 
> René J.V. Bertin wrote:
> 1: why should one care? It is said nowhere that the setting defined as 
> "show icons on buttons" in `systemsettings(5)` applies only to dialogs. 
> Rather, the tooltip says "when this is enabled, KDE applications will show 
> icons alongside [sic!] some important buttons".
> In other words, when "this" is *not* enabled, there should be no icons, 
> period.
> I have found no sign in the code that the ShowIconsOnPushButtons hint is 
> to be used only for dialogs; `SH_DialogButtonBox_ButtonsHaveIcons` indeed 
> carries the suggestion in its name but I would not be surprised if Qt really 
> thinks of it in a more general sense. Probably also because the notion of 
> what a dialog is has become a lot vaguer.
> 
> And that also happens to be what Qt does; buttons show their icons on 
> Linux (and other Unix variants?) but on OS X or MS Windows displaying of 
> those icons is deactivated unless you use a style that enables it. In fact, 
> the default setting for `SH_DialogButtonBox_ButtonsHaveIcons` is false except 
> in the generic Unix theme (= it does work globally like 
> `AA_DontShowIconsInMenus` everywhere else).
> 
> 2: a user who indicates s/he doesn't want to see icons will get an empty 
> button. That's also what can happen with QPushButton, and app developers have 
> to take this into consideration. Cf. toolbars (and Qt's position on the use 
> of "texted separators").
> I don't think I've ever come across a standard button showing only an 
> icon, except possibly the arrow button next to the progress indicators in 
> KMail and KDevelop.
> 
> As to fixing it here: as it turns out, "here" is the main source for 
> annoying icons rearing their silly heads on buttons on my screen ;) and it 
> was also something of a challenge to understand why they kept appearing 
> despite the fact that all code appeared to return the value of 
> `ShowIconsOnPushButtons`. Deprecated or not, it doesn't look like all 
> applications are going to stop using it anytime soon.
> 
> I looked into fixing the issue in KDialogButtonBox but saw that it does 
> nothing to override the `ShowIconsOnPushButtons` setting. The only way to 
> respect the setting through that class (or a modern equivalent) would be to 
> set an empty icon if `ShowIconsOnPushButtons=false`. That introduces another 
> regression: changes in this setting are supposed to be reflected by running 
> applications without requiring a restart (or a recreation of dialogs). If it 
> were just me I'd decree that buttons can have either text or an icon, but 
> right now we have to make do with this mixed situation.
> 
> I don't mind making this an OS X (and MS Windows) specific modification, 
> of course, but on those platforms
> 
> Thomas Lübking wrote:
> > 1: why should one care?
> 
> Because, as explained, that is what the hint says. Nothing else.
> 
> > I have found no sign in the code that the ShowIconsOnPushButtons hint 
> is to be used only for dialogs
> 
> No, but it's been used to feed SH_DialogButtonBox_ButtonsHaveIcons *AND* 
> KPushButton.
> ShowIconsOnPushButtons implied SH_DialogButtonBox_ButtonsHaveIcons but 
> NOT vv.
> 
> The approach is wrong, since you're abusing the hint for generalisation.
> 
> > but on OS X or MS Windows
> 
> ... Qt uses native elements which might simply globally downforce the 
> pushbutton icon nonsense (as could any style - I was more than once close to 
> doing that in virtuality)
> Eg. Breeze might do that on favor of the HIG, but that's not relevant 
> here.
> 
> Downforcing in KPushButton means to 

Re: Review Request 126308: KPushButton: respect SH_DialogButtonBox_ButtonsHaveIcons, also when drawing

[René J . V . Bertin]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126308/
---

(Updated Dec. 11, 2015, 5:26 p.m.)


Review request for KDE Software on Mac OS X, KDE Frameworks, Qt KDE, Hugo 
Pereira Da Costa, and Yichao Yu.


Changes
---

Thomas, what exactly did you mean with "QDialogButtonBox::addButton should do 
correctly"? Looking at Qt's code again, I can confirm that 
QDB::addButton(StandardButton) is the only one invoking the private 
createButton method which in turn sets an icon if ButtonsHaveIcons is true. The 
other QDB::addButton methods simply call the private addButton method which 
will do a layout step by default, but I don't see where an icon would be added.

Should I understand that `style->standardIcon()` is invoked during drawing as a 
function of button's role?

I cannot find evidence of that in QtCurve. But if that is the case nonetheless, 
why are we patching KDialogButtonBox again? And how do you explain that 
removing the icon after a button is created has the effect it has (when the 
style will continue to see the button role)?


Repository: kdelibs4support


Description
---

KF5 applications have long had a habit of drawing icons on buttons even when 
this feature was turned off in the user's setting. This was mostly noticeable 
in applications built on kdelibs4support.

It seems that the actual culprit is in Qt's QPushButton implementation 
(https://bugreports.qt.io/browse/QTBUG-49887), but it is possible to work 
around it in `KPushButton::paintEvent`, by removing the icon (forcing it to the 
null icon) in the option instance, before handing off control to the painter.


Diffs (updated)
-

  src/kdeui/kdialogbuttonbox.cpp 0f6649b 
  src/kdeui/kpushbutton.cpp 98534fa 

Diff: https://git.reviewboard.kde.org/r/126308/diff/


Testing
---

On Kubuntu 14.04 and OS X 10.9.5 with Qt 5.5.1 and KF5 frameworks 5.16.0 .

I have not yet verified if there are other classes where this modification 
would be relevant too.


Thanks,

René J.V. Bertin

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126308: KPushButton: respect SH_DialogButtonBox_ButtonsHaveIcons, also when drawing

[René J . V . Bertin]

> On Dec. 11, 2015, 2:55 p.m., Thomas Lübking wrote:
> > src/kdeui/kdialogbuttonbox.cpp, line 61
> > 
> >
> > unrelated and it won't leak, since the cleanup is done by the 
> > parent/child relation ("this" passed to KPushButton)

Maybe it won't leak, but the question remains why what buttons with an invalid 
role are good for.


- René J.V.


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126308/#review89351
---


On Dec. 11, 2015, 3:03 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126308/
> ---
> 
> (Updated Dec. 11, 2015, 3:03 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X, KDE Frameworks, Qt KDE, Hugo 
> Pereira Da Costa, and Yichao Yu.
> 
> 
> Repository: kdelibs4support
> 
> 
> Description
> ---
> 
> KF5 applications have long had a habit of drawing icons on buttons even when 
> this feature was turned off in the user's setting. This was mostly noticeable 
> in applications built on kdelibs4support.
> 
> It seems that the actual culprit is in Qt's QPushButton implementation 
> (https://bugreports.qt.io/browse/QTBUG-49887), but it is possible to work 
> around it in `KPushButton::paintEvent`, by removing the icon (forcing it to 
> the null icon) in the option instance, before handing off control to the 
> painter.
> 
> 
> Diffs
> -
> 
>   src/kdeui/kdialogbuttonbox.cpp 0f6649b 
>   src/kdeui/kpushbutton.cpp 98534fa 
> 
> Diff: https://git.reviewboard.kde.org/r/126308/diff/
> 
> 
> Testing
> ---
> 
> On Kubuntu 14.04 and OS X 10.9.5 with Qt 5.5.1 and KF5 frameworks 5.16.0 .
> 
> I have not yet verified if there are other classes where this modification 
> would be relevant too.
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126308: KPushButton: respect SH_DialogButtonBox_ButtonsHaveIcons, also when drawing

[René J . V . Bertin]

> On Dec. 11, 2015, 2:55 p.m., Thomas Lübking wrote:
> > src/kdeui/kdialogbuttonbox.cpp, line 36
> > 
> >
> > unrelated and not required

Not required indeed, but related in the sense that it removes any ambiguity on 
what method is being called ;)


> On Dec. 11, 2015, 2:55 p.m., Thomas Lübking wrote:
> > src/kdeui/kdialogbuttonbox.cpp, line 39
> > 
> >
> > QDialogButtonBox::addButton should do correctly anyway, so please don't 
> > workaround things that are not broken.

No, I've looked at Qt 5.5.1 . The only QDialogButtonBox::addButton that "does 
correctly" is the one that takes a StandardButton. I haven't had time to test 
this (will need to rebuild QtBase first) but I'm pretty sure that that method 
creates a button with an icon with its sequence

```
QPushButton *button = new QPushButton(text, this);
d->addButton(button, role);
```

My approach here is to avoid adding an icon if ButtonsHaveIcons is false, or 
remove the icon if one was already added. That's what QDB does with its 
::addButton(StandardButton btn) method (calling a private createButton() 
method). Any other approach is useless without a style supporting and enforcing 
ButtonsHaveIcons, but which such a style KDialogButtonBox doesn't need to be 
fixed in the first place...


> On Dec. 11, 2015, 2:55 p.m., Thomas Lübking wrote:
> > src/kdeui/kdialogbuttonbox.cpp, line 57
> > 
> >
> > you can completely spare this, there's no reason to manipulate a copy 
> > of the GuiItem, just burns CPU

In that case I'll have to remove the `const` from guiitem, meaning a change to 
the API.


> On Dec. 11, 2015, 2:55 p.m., Thomas Lübking wrote:
> > src/kdeui/kdialogbuttonbox.cpp, line 70
> > 
> >
> > Setting the icon is sufficient, please do not mess around with other 
> > attributes.

Are you sure? setIcon() doesn't call setIconSize() nor does it reset any size 
information already present. Is it a good idea to replace an icon and leaving 
the size information from the previous icon)? NB: should the icon from the 
KGuiItem override the role's standard icon or should it be the other way round 
(provided icon as a default when the role doesn't provide an icon, for 
instance)?


> On Dec. 11, 2015, 2:55 p.m., Thomas Lübking wrote:
> > src/kdeui/kdialogbuttonbox.cpp, line 75
> > 
> >
> > this is really the only thing you should need to do here.

Cf. the previous comment about icon priority: this method can provide 2 icons 
that the button will have to chose from.

And I think that it's probably a good idea to set the icon size to 0 when the 
intent is to remove the icon completely.


> On Dec. 11, 2015, 2:55 p.m., Thomas Lübking wrote:
> > src/kdeui/kpushbutton.cpp, line 257
> > 
> >
> > still wrong and again, please don't mess with the icon size - you're 
> > just tempting DIV zero segfaults.

what?? Code that doesn't check integer div 0 should be encouraged to crash. A 
different bug than the one we're addressing here, but not one I have any 
patience with/for.

I could use QSize() instead of QSize(0,0); the former would mean 
iconSize.isValid() will return false while with the latter it'll return true. 
But note that functions like QPushButton::sizeHint() do not check isValid. A 
bit of a conundrum.
Am I right that a button that never had an icon will have `iconSize() == 
QSize()` ?


- René J.V.


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126308/#review89351
---


On Dec. 11, 2015, 3:03 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126308/
> ---
> 
> (Updated Dec. 11, 2015, 3:03 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X, KDE Frameworks, Qt KDE, Hugo 
> Pereira Da Costa, and Yichao Yu.
> 
> 
> Repository: kdelibs4support
> 
> 
> Description
> ---
> 
> KF5 applications have long had a habit of drawing icons on buttons even when 
> this feature was turned off in the user's setting. This was mostly noticeable 
> in applications built on kdelibs4support.
> 
> It seems that the actual culprit is in Qt's QPushButton implementation 
> (https://bugreports.qt.io/browse/QTBUG-49887), but it is possible to work 
> around it in `KPushButton::paintEvent`, by removing the icon 

kwidgetsaddons on OS X: menus become menu items

[René J . V . Bertin]

Hi,

After building kate I'm seeing repeating messages like the ones below on the 
calling terminal, using either Kate or KWrite. This also happens with 
Christoph's bundle(d) build. (I've patched my Qt build to be more informative 
though).

void QCocoaMenu::insertNative(QCocoaMenuItem *, QCocoaMenuItem *) Menu item 
"Clipboard " is already in menu "Edit" , remove it from the other menu 
first before inserting into "Untitled"
void QCocoaMenu::insertNative(QCocoaMenuItem *, QCocoaMenuItem *) Menu item 
"" is already in menu "" , remove it from the other menu first before 
inserting into "Untitled"
virtual void QCocoaMenu::removeMenuItem(QPlatformMenuItem *) Item "Clipboard 
" to remove does not belong to this menu "Untitled"
virtual void QCocoaMenu::removeMenuItem(QPlatformMenuItem *) Item "" 
to remove does not belong to this menu "Untitled"
void QCocoaMenu::insertNative(QCocoaMenuItem *, QCocoaMenuItem *) Menu item 
"Clipboard " is already in menu "Edit" , remove it from the other menu 
first before inserting into "Untitled"
void QCocoaMenu::insertNative(QCocoaMenuItem *, QCocoaMenuItem *) Menu item 
"" is already in menu "" , remove it from the other menu first before 
inserting into "Untitled"
void QCocoaMenu::insertNative(QCocoaMenuItem *, QCocoaMenuItem *) Menu item 
"Clipboard " is already in menu "Edit" after item "" , remove it from 
the other menu first before inserting into "Edit"
virtual void QCocoaMenu::removeMenuItem(QPlatformMenuItem *) Item "Clipboard 
" to remove does not belong to this menu "Untitled"
virtual void QCocoaMenu::removeMenuItem(QPlatformMenuItem *) Item "" 
to remove does not belong to this menu "Untitled"

From what I've been able to gather, this means that the 2 menus in question get 
added to another menu (the Edit menu for "Clipboard History") rather than 
becoming a (toplevel) menu of their own.
Evidently it is too late to figure out why this happens at the time the warning 
is printed.

Qt has long had a "feature" in its menu item handling on OS X where it would 
guess the menuRole from the action text. That feature is intended to let 
certain QActions become "menu items" in the menu that's supposed to hold them 
on OS X, without requiring any action from the developer. So a QAction called 
Preferences, Settings or matching a few other patterns will go into the 
"Application menu" rather than into the menu the application is expecting, and 
be called Preferences.
There is only 1 way around this: call setMenuRole(QAction::NoRole) on each 
QAction before it can be added to a menu, or use the intended MenuRole for 
QActions. 

Either way, I have been trying set QAction::NoRole in a number of places that 
seemed relevant (up to the KActionMenu ctors), to no avail. The "Clipboard 
History" menu is present in the Edit menu where it is actually not out of place 
(and is functional), but the Bookmarks menu is nowhere to be found.

I could use a hand with this. If no better solution can be found I'd prefer 
deactivating the corresponding features (on OS X) rather than getting this 
terminal pollution and possible application instability (I've already seen 
cases where the menus wouldn't open).

Thanks,
René
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: kwidgetsaddons on OS X: menus become menu items

[René J . V . Bertin]

On Friday December 11 2015 23:55:35 René J.V. Bertin wrote:

KActionMenu being derived from QWidgetAction I wonder if the following blurb 
from the QWidgetAction documentation is relevant here:

OS X: If you add a widget to a menu in the application's menu bar on OS X, the 
widget will be added and it will function but with some limitations:

The widget is reparented away from the QMenu to the native menu view. If you 
show the menu in some other place (e.g. as a popup menu), the widget will not 
be there.


R.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Review Request 126314: Port klauncher to xcb

[Martin Gräßlin]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126314/
---

Review request for KDE Frameworks and David Faure.


Repository: kinit


Description
---

Port klauncher to xcb


Diffs
-

  src/klauncher/klauncher.cpp 8b3d343b9fb2c852ea2d6c559f13c96a0467d72b 
  src/klauncher/CMakeLists.txt 746edfac0b840aa033be219ae5d094689006db6f 
  src/klauncher/klauncher.h e155f72d76d4f99172646ab797ec8c4dd006ddf7 

Diff: https://git.reviewboard.kde.org/r/126314/diff/


Testing
---


Thanks,

Martin Gräßlin

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126320: Read KPluginMetada's property X-Plasma-ComponentTypes as a stringlist

[Hrvoje Senjan]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126320/#review89369
---


i had the similar problem after commit f35e514 (only empty panel would be 
shown). this patch fixes the problem here.

- Hrvoje Senjan


On Dec. 11, 2015, 7:48 p.m., Martin Klapetek wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126320/
> ---
> 
> (Updated Dec. 11, 2015, 7:48 p.m.)
> 
> 
> Review request for KDE Frameworks, Plasma and Alex Richardson.
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> ---
> 
> plasma-scriptengine.desktop defines the property "X-Plasma-ComponentTypes"
> as Type=QStringList. When reading it using KPluginMetaData::value(..) it
> expects a QString back. This used to work but regressed in kcoreaddons in
> commit cfd18cf09b559a050fd6a2680ad4e71eeb950383. Now I'm not sure if calling
> KPluginMetaData::value(..) on a property that is known to be a stringlist
> should actually return a QString (Alex?), but making it read the property
> as a stringlist works and is correct and also fixes Plasma startup for me.
> 
> 
> Diffs
> -
> 
>   src/plasma/scripting/scriptengine.cpp 1b132de 
> 
> Diff: https://git.reviewboard.kde.org/r/126320/diff/
> 
> 
> Testing
> ---
> 
> Plasma would get stuck on startup, now I can run Plasma again.
> 
> 
> Thanks,
> 
> Martin Klapetek
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Jenkins-kde-ci: threadweaver master kf5-qt5 » Linux,gcc - Build # 23 - Fixed!

[no-reply]

GENERAL INFO

BUILD SUCCESS
Build URL: 
https://build.kde.org/job/threadweaver%20master%20kf5-qt5/PLATFORM=Linux,compiler=gcc/23/
Project: PLATFORM=Linux,compiler=gcc
Date of build: Fri, 11 Dec 2015 17:28:27 +
Build duration: 1 min 24 sec

CHANGE SET
Revision 8490cbf0e03ade567357809c27699059720709f4 by Alex Richardson: (Fix typo 
that broke the build)
  change: edit examples/HelloInternet/CMakeLists.txt


JUNIT RESULTS

Name: (root) Failed: 0 test(s), Passed: 7 test(s), Skipped: 0 test(s), Total: 7 
test(s)

COBERTURA RESULTS

Cobertura Coverage Report
  PACKAGES 2/2 (100%)FILES 70/78 (90%)CLASSES 70/78 (90%)LINE 2536/2766 
(92%)CONDITIONAL 3383/6514 (52%)

By packages
  
autotests
FILES 19/19 (100%)CLASSES 19/19 (100%)LINE 1270/1275 
(100%)CONDITIONAL 2537/5016 (51%)
src
FILES 51/59 (86%)CLASSES 51/59 (86%)LINE 1266/1491 
(85%)CONDITIONAL 846/1498 (56%)___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Jenkins-kde-ci: threadweaver master kf5-qt5 » Linux,gcc - Build # 23 - Fixed!

[no-reply]

GENERAL INFO

BUILD SUCCESS
Build URL: 
https://build.kde.org/job/threadweaver%20master%20kf5-qt5/PLATFORM=Linux,compiler=gcc/23/
Project: PLATFORM=Linux,compiler=gcc
Date of build: Fri, 11 Dec 2015 17:28:27 +
Build duration: 1 min 24 sec

CHANGE SET
Revision 8490cbf0e03ade567357809c27699059720709f4 by Alex Richardson: (Fix typo 
that broke the build)
  change: edit examples/HelloInternet/CMakeLists.txt


JUNIT RESULTS

Name: (root) Failed: 0 test(s), Passed: 7 test(s), Skipped: 0 test(s), Total: 7 
test(s)

COBERTURA RESULTS

Cobertura Coverage Report
  PACKAGES 2/2 (100%)FILES 70/78 (90%)CLASSES 70/78 (90%)LINE 2536/2766 
(92%)CONDITIONAL 3383/6514 (52%)

By packages
  
autotests
FILES 19/19 (100%)CLASSES 19/19 (100%)LINE 1270/1275 
(100%)CONDITIONAL 2537/5016 (51%)
src
FILES 51/59 (86%)CLASSES 51/59 (86%)LINE 1266/1491 
(85%)CONDITIONAL 846/1498 (56%)___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Jenkins-kde-ci: kinit master stable-kf5-qt5 » Linux,gcc - Build # 28 - Failure!

[no-reply]

GENERAL INFO

BUILD FAILURE
Build URL: 
https://build.kde.org/job/kinit%20master%20stable-kf5-qt5/PLATFORM=Linux,compiler=gcc/28/
Project: PLATFORM=Linux,compiler=gcc
Date of build: Fri, 11 Dec 2015 17:28:27 +
Build duration: 56 sec

CHANGE SET
No changes
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

kdebugdialog5 crashing on exit (FYI)

[René J . V . Bertin]

Hi,

I mentioned having crashes on exit with kdebugdialog5 a couple of days ago, 
deep 
in Qt territory. At first I thought the culprit was my implementation of a 
"KdeMacPlatformTheme" that inherits from the regular KdePlatformTheme.

Then I noticed that kdebugdialog5 also crashes on exit on Linux, when the 
KdePlatformTheme isn't present but the app is started with --fullmode (and does 
the same on OS X with the native platform theme).

Thiago Macieira referred me to a proposed change to refrain from unloading 
plugins (https://codereview.qt-project.org/#/c/140750/) and I can report that 
applying the patch indeed solves the issue for me (on OS X).

R.

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126246: Add test for dynamically changing file definitions

[Marco Martin]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126246/
---

(Updated Dec. 11, 2015, 6:03 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks and Plasma.


Changes
---

Submitted with commit e3d27e00c6ad4428b59188058a10e1864f85 by Marco Martin 
to branch master.


Repository: kpackage


Description
---

this, referred to https://git.reviewboard.kde.org/r/126244/ tests that adding 
or removing a file definition depending on the path (and whatever criteria, 
like metadata contents) works. since is already done in several places has to 
work correctly


Diffs
-

  autotests/packagestructuretest.h de2038e 
  autotests/packagestructuretest.cpp 4784bfd 

Diff: https://git.reviewboard.kde.org/r/126246/diff/


Testing
---


Thanks,

Marco Martin

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126308: KPushButton: respect SH_DialogButtonBox_ButtonsHaveIcons, also when drawing

[René J . V . Bertin]

> On Dec. 10, 2015, 11:11 p.m., Thomas Lübking wrote:
> > 1. What tells you that this is a dialog buttonbox pushbutton?
> > 2. What happens if the button has no text?
> > 
> > 
> > The bug is in QDialogButtonBox (or rather the K variant, 
> > QDialogButtonBoxPrivate::createButton() seems to incorporate the hint 
> > correctly)
> > 
> > 
> > [KDE] has "ShowIconsOnPushButtons=false" in kdeglobals which was 
> > interpreted by KPushButton _and_ kstyle for the hint, but the hint only 
> > covers Q'DialogButtonBox'es - there's simply no global rule for this like 
> > AA_DontShowIconsInMenus
> > 
> > => KDialogButtonBox shall respect the hint for its buttons (there're two 
> > special creation routines).
> > 
> > For the rest, the platform plugin ideally picks the kdeglobals setting and 
> > exports it to the application object (dyn property?) where the style can 
> > pick it and incorporate it into its calculations (ie. if no icons are 
> > wanted and there's text or arrow, omit the icon in size calculation and 
> > painting)
> > 
> > "Fixing" that in deprecated KPushButton doesn't really fix anything. We'll 
> > face the mix we had, just that users of QPushButton were far less prone to 
> > pass them an icon in pre-KF5 times.
> > 
> > Please also attach Hugo Pereira Da Costa.
> 
> René J.V. Bertin wrote:
> 1: why should one care? It is said nowhere that the setting defined as 
> "show icons on buttons" in `systemsettings(5)` applies only to dialogs. 
> Rather, the tooltip says "when this is enabled, KDE applications will show 
> icons alongside [sic!] some important buttons".
> In other words, when "this" is *not* enabled, there should be no icons, 
> period.
> I have found no sign in the code that the ShowIconsOnPushButtons hint is 
> to be used only for dialogs; `SH_DialogButtonBox_ButtonsHaveIcons` indeed 
> carries the suggestion in its name but I would not be surprised if Qt really 
> thinks of it in a more general sense. Probably also because the notion of 
> what a dialog is has become a lot vaguer.
> 
> And that also happens to be what Qt does; buttons show their icons on 
> Linux (and other Unix variants?) but on OS X or MS Windows displaying of 
> those icons is deactivated unless you use a style that enables it. In fact, 
> the default setting for `SH_DialogButtonBox_ButtonsHaveIcons` is false except 
> in the generic Unix theme (= it does work globally like 
> `AA_DontShowIconsInMenus` everywhere else).
> 
> 2: a user who indicates s/he doesn't want to see icons will get an empty 
> button. That's also what can happen with QPushButton, and app developers have 
> to take this into consideration. Cf. toolbars (and Qt's position on the use 
> of "texted separators").
> I don't think I've ever come across a standard button showing only an 
> icon, except possibly the arrow button next to the progress indicators in 
> KMail and KDevelop.
> 
> As to fixing it here: as it turns out, "here" is the main source for 
> annoying icons rearing their silly heads on buttons on my screen ;) and it 
> was also something of a challenge to understand why they kept appearing 
> despite the fact that all code appeared to return the value of 
> `ShowIconsOnPushButtons`. Deprecated or not, it doesn't look like all 
> applications are going to stop using it anytime soon.
> 
> I looked into fixing the issue in KDialogButtonBox but saw that it does 
> nothing to override the `ShowIconsOnPushButtons` setting. The only way to 
> respect the setting through that class (or a modern equivalent) would be to 
> set an empty icon if `ShowIconsOnPushButtons=false`. That introduces another 
> regression: changes in this setting are supposed to be reflected by running 
> applications without requiring a restart (or a recreation of dialogs). If it 
> were just me I'd decree that buttons can have either text or an icon, but 
> right now we have to make do with this mixed situation.
> 
> I don't mind making this an OS X (and MS Windows) specific modification, 
> of course, but on those platforms
> 
> Thomas Lübking wrote:
> > 1: why should one care?
> 
> Because, as explained, that is what the hint says. Nothing else.
> 
> > I have found no sign in the code that the ShowIconsOnPushButtons hint 
> is to be used only for dialogs
> 
> No, but it's been used to feed SH_DialogButtonBox_ButtonsHaveIcons *AND* 
> KPushButton.
> ShowIconsOnPushButtons implied SH_DialogButtonBox_ButtonsHaveIcons but 
> NOT vv.
> 
> The approach is wrong, since you're abusing the hint for generalisation.
> 
> > but on OS X or MS Windows
> 
> ... Qt uses native elements which might simply globally downforce the 
> pushbutton icon nonsense (as could any style - I was more than once close to 
> doing that in virtuality)
> Eg. Breeze might do that on favor of the HIG, but that's not relevant 
> here.
> 
> Downforcing in KPushButton means to 

Re: system tray test?

[René J . V . Bertin]

Sebastian Kügler wrote:

> Not sure what exactly you mean with "putting up and controlling an icon in the
> system tray", but we do have a status notifier test app, you can find it in:
> plasma-workspace/applets/systemtray/tests/statusnotifier

Heh, that must be because I asked the question in one of those "I should be 
doing something else right now, but I'd better handle this lest I forget it 
again" moments. I referred to KSystemTrayIcon, and I see that it's being 
replaced with KStatusNotifierItem.

> There are also some test programs in frameworks/knotifications/tests, you may
> want to look at.

Indeed. Thanks for pointing that out. kstatusnotifieritemtest mostly works. 
There are some complaints about missing icons though.

And I see there is the platform-specific issue with the tray menu's title item:

titleAction = m->addSection(qApp->windowIcon(), title);

will only add a separator on platforms that don't support texted separators 
(basically all except for X11 and maybe Wayland). Qt's position on this is 
"don't use addSection (unless you don't care about the text getting lost)" but 
I 
think this is an application where the text should definitely *not* get lost.

I'd suggest replacing the line above with a sequence "m->addAction, m-
>addSeparator" on OS X, or do you have other suggestions?

R



___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Review Request 126320: Read KPluginMetada's property X-Plasma-ComponentTypes as a stringlist

[Martin Klapetek]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126320/
---

Review request for KDE Frameworks, Plasma and Alex Richardson.


Repository: plasma-framework


Description
---

plasma-scriptengine.desktop defines the property "X-Plasma-ComponentTypes"
as Type=QStringList. When reading it using KPluginMetaData::value(..) it
expects a QString back. This used to work but regressed in kcoreaddons in
commit cfd18cf09b559a050fd6a2680ad4e71eeb950383. Now I'm not sure if calling
KPluginMetaData::value(..) on a property that is known to be a stringlist
should actually return a QString (Alex?), but making it read the property
as a stringlist works and is correct and also fixes Plasma startup for me.


Diffs
-

  src/plasma/scripting/scriptengine.cpp 1b132de 

Diff: https://git.reviewboard.kde.org/r/126320/diff/


Testing
---

Plasma would get stuck on startup, now I can run Plasma again.


Thanks,

Martin Klapetek

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel