Review Request 127809: [Platform xcb] Get best icon size when he's not specified

[Anthony Fieroni]

> On Май 2, 2016, 5:02 след обяд, Martin Gräßlin wrote:
> > What is the "best" size if it's not specified? Why is your change better than how it was?
> > 
> > This change adjusts a very important part of the icon lookup functionality used by KWin and Plasma. I'm a little bit scared of touching it.
> > 
> > The change does not come with a test, so I have problems understanding what you want to change. Could you please extend the tests to ensure that this does not break existing functionality?
> 
> Anthony Fieroni wrote:
>     I, personally, don't know when NETWinInfo->icon or icccmIconPixmap can fail but when it happend function returns icon with size 16x16 when is called like this -> KWindowSystem::icon (wid);
> 
> Martin Gräßlin wrote:
>     well that's what we have unit test for. They can show us whether the code fails.

xprop of dragonplayer

_NET_WM_ICON_GEOMETRY(CARDINAL) = 674, 736, 164, 30
_NET_WM_ALLOWED_ACTIONS(ATOM) = _NET_WM_ACTION_MOVE, _NET_WM_ACTION_RESIZE, _NET_WM_ACTION_MINIMIZE, _NET_WM_ACTION_SHADE, _NET_WM_ACTION_MAXIMIZE_VERT, _NET_WM_ACTION_MAXIMIZE_HORZ, _NET_WM_ACTION_FULLSCREEN, _NET_WM_ACTION_CHANGE_DESKTOP, _NET_WM_ACTION_CLOSE
_KDE_NET_WM_FRAME_STRUT(CARDINAL) = 2, 2, 24, 4
_NET_FRAME_EXTENTS(CARDINAL) = 2, 2, 24, 4
_NET_WM_DESKTOP(CARDINAL) = 0
_KDE_NET_WM_ACTIVITIES(STRING) = "153b4652-e23a-44c6-baf7-e7a0d812498a"
WM_STATE(WM_STATE):
                window state: Normal
                icon window: 0x0
_NET_WM_STATE(ATOM) = 
_NET_WM_ICON_NAME(UTF8_STRING) = 
XdndAware(ATOM) = BITMAP
WM_NAME(STRING) = 
_NET_WM_NAME(UTF8_STRING) = "???????? ?? ????? Dragon"
_KDE_NET_WM_USER_CREATION_TIME(CARDINAL) = 49919374
_MOTIF_WM_HINTS(_MOTIF_WM_HINTS) = 0x3, 0x3e, 0x7e, 0x0, 0x0
_NET_WM_WINDOW_TYPE(ATOM) = _NET_WM_WINDOW_TYPE_NORMAL
_XEMBED_INFO(_XEMBED_INFO) = 0x0, 0x1
WM_CLIENT_LEADER(WINDOW): window id # 0x6c00006
WM_HINTS(WM_HINTS):
                Client accepts input or input focus: True
                Initial state is Normal State.
_NET_WM_PID(CARDINAL) = 917
_NET_WM_SYNC_REQUEST_COUNTER(CARDINAL) = 113246213
WM_CLASS(STRING) = "dragon", "dragonplayer"
WM_PROTOCOLS(ATOM): protocols  WM_DELETE_WINDOW, WM_TAKE_FOCUS, _NET_WM_PING, _NET_WM_SYNC_REQUEST
WM_NORMAL_HINTS(WM_SIZE_HINTS):
                user specified size: 651 by 295
                program specified minimum size: 651 by 295
                window gravity: Static

NETWinInfo icon and icccmIconPixmap fails, i can add such a test, if it not present.
So http://api.kde.org/frameworks-api/frameworks5-apidocs/kwindowsystem/html/netwm_8cpp_source.html#l03544 NETWinInfo trys to return the larger one, so this function must be identical, at functionality


- Anthony


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


On Май 2, 2016, 5:04 след обяд, Anthony Fieroni wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127809/
> -----------------------------------------------------------
> 
> (Updated Май 2, 2016, 5:04 след обяд)
> 
> 
> Review request for KDE Frameworks, KDE Usability and Martin Gräßlin.
> 
> 
> Bugs: 362324
>     https://bugs.kde.org/show_bug.cgi?id=362324
> 
> 
> Repository: kwindowsystem
> 
> 
> Description
> -------
> 
> The api function is KWindowSystem::icon (WId win, int width=-1, int height=-1, bool scale=false) so caller must get best size not worst when width/height is not specified.
> 
> 
> Diffs
> -----
> 
>   src/platforms/xcb/kwindowsystem.cpp 5b7c65a 
> 
> Diff: https://git.reviewboard.kde.org/r/127809/diff/
> 
> 
> Testing
> -------
> 
> 
> File Attachments
> ----------------
> 
> before
>   https://git.reviewboard.kde.org/media/uploaded/files/2016/05/01/6d718ef6-26cf-4866-94d2-4ffbdfc906fe__Screenshot_20160426_232109.png
> after
>   https://git.reviewboard.kde.org/media/uploaded/files/2016/05/01/7dcab4ae-e451-4d43-8799-a0fcab471a3d__Screenshot_20160501_224642.png
> 
> 
> Thanks,
> 
> Anthony Fieroni
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20160503/9559bd0a/attachment-0001.html>