View Revision
hpereiradacosta added a comment.

In D3131#59715, @hpereiradacosta wrote:

Hi Marco,
Two problems with the commit:
1/ it does not compile under kde4 (and we are still maintaining a kde4 and a kf5 version)

breezestyle.cpp:6652:26: error: ‘const class QStyleOptionSlider’ has no member named ‘styleObject’

else if( option->styleObject ) widgetMouseOver = option->styleObject->property("hover").toBool();

I can easily fix that.

2/ there are warnings:

/home/hpereira/kf5/src/kde/workspace/breeze/kstyle/breezestyle.cpp:6719:24: warning: unused variable ‘mouseOver’ [-Wunused-variable]

const bool mouseOver( ( option->state & State_MouseOver ) );
           ^

/home/hpereira/kf5/src/kde/workspace/breeze/kstyle/breezestyle.cpp:6724:18: warning: ‘widgetMouseOver’ may be used uninitialized in this function [-Wmaybe-uninitialized]

else if( !widgetMouseOver ) return Qt::transparent;

Here I think this is because the first mouseOver should be widgetMouseOver, but I am not sure. Can you double check ?

Looking more at the code, I think I am confused about the logic ...
How is widgetMouseOver different from mouseOver later on.
Why is one calling widget->mouseOver() and the second option->state & State_MouseOver ? (I would think the second is correct and should be used everywhere)

And finally, about the block on "disabled" state scrollbars, do we want the groove to show up on mouse-over in that case ? I think this tends to indicate that something will happen if you click, which is not the case ...

Sorry if I have not commented on this before the commit, I guess I must have not reviewed this part of the patch carefully enough.


REPOSITORY
rBREEZE Breeze

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

EMAIL PREFERENCES
https://phabricator.kde.org/settings/panel/emailpreferences/

To: mart, Plasma, Breeze, VDG, hpereiradacosta
Cc: plasma-devel, abetts, lesliezhai, ali-mohamed, jensreuterberg, sebas