> On May 2, 2016, 4:02 p.m., 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. > > Anthony Fieroni wrote: > 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
Why do you show me the xprop of dragonplayer? What do you want to show me? I really have a hard problem understanding why you want to change the behavior. Especially as the API documentation doesn't say anything about what it does when width/height are not specified. We might break existing code here! That's why I need to understand the motivation behind the change. I need to understand why the change is better than the current one. Especially we need to exclude the possiblity of a misuse of the API. Given the xprop there is no proper icon set on the window. This is a bug in dragon player. - Martin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127809/#review95084 ----------------------------------------------------------- On May 2, 2016, 4:04 p.m., Anthony Fieroni wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/127809/ > ----------------------------------------------------------- > > (Updated May 2, 2016, 4:04 p.m.) > > > 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 > >
_______________________________________________ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
