On Thu, 29 Nov 2012, BALATON Zoltan escribió:
> On Thu, 29 Nov 2012, Rodolfo Garc?a Pe?as (kix) wrote:
> >Yes, you are right. We have four options:
> >
> >1. ignore flag + file set
> >2. no ignore flag + file set
> >3. ignore flag + no file set
> >4. no ignore flag + no file set
> >
> >Option 1 is clear, we provide a new icon and ignore the supplied
> >icon by the client.
> >Option 4 is clear, we use the client supplied icon and the icon
> >provided is not used.
> >
> >But, what about options 2 and 3?
>
> Option 3 is fairly clear too. If there is no user set icon you
> either ignore the Ignore flag (bacause it makes no sense) or you may
> use default app icon to override the icon window but I'm not sure
> how useful is that.
>
> For option 2 I already said what I think should happen: use client
> supplied icon _window_ or user supplied icon otherwise.
>
> >If we provide icon, should the flag ignored?
>
> No, there's a reason why this flag is there. If it could be ignored
> there would not be such a flag.
>
> >If we check the flag, should the file ignored?
>
> No, the flag says AlwaysUserIcon. (That is ignore the client
> supplied icons of any sorts and always use the icon file instead.)
>
> >Should we can remove the check? is the work done by the text
> >field? If the text is empty, should the icon provided by the
> >client used?
>
> But why should we change anything? Could you just make everything do
> the same that it used to do before you've started changing it? Your
> patches were meant to be clean up so they should preserve
> functionality and not change behaviour. Or did I get that wrong? Was
> there a problem you've tried to fix other than cleaning up code?
>
> >IMO we are doing something wrong with these applications (like
> >xfig). Why these applications have that behaviour? Xfig don't use
> >title in the miniwindow, sets their own icon,...
>
> I think it is because the Ignore client supplied icon flag is broken
> and not used even if it's set in WMWindowAttributes. Since xfig
> provides an icon window it will override the icon even when the
> Ignore flag is set (or even if the AlwaysUserIcon flag is set in the
> config because the checkbox in the GUI is also detached from what's
> in the config file).
>
> >Again, if we remove these lines in create_for_wwindow (icon.c)
> >xfig runs like other applications. IMHO, these lines should be
> >removed:
> >
> >121 if (wwin->wm_hints && (wwin->wm_hints->flags & IconWindowHint)) {
> >122 if (wwin->client_win == wwin->main_window) {
> >123 WApplication *wapp;
> >124 /* do not let miniwindow steal app-icon's
> >icon window */
> >125 wapp = wApplicationOf(wwin->client_win);
> >126 if (!wapp || wapp->app_icon == NULL)
> >127 icon->icon_win = wwin->wm_hints->icon_window;
> >128 } else {
> >129 icon->icon_win = wwin->wm_hints->icon_window;
> >130 }
> >131 }
> >
> >Try it, and you will see that xfig has a common behaviour. The
> >application has icon, the title,... These lines override the
> >common icon management.
>
> But this breaks the wmicontest test case I've sent earlier and
> probably also dockapps which need to have an icon window.
>
> >Probably we should check the icon behaviour to see if the current
> >code includes the idea of these lines, but, IMO this lines breaks
> >the flow for icon management, creating a shortcut with a different
> >flow and should be removed.
>
> I don't get this. Are you trying to get rid of a special case based
> only on aesthetic reasons? There is a reason that this special case
> is there (as stated in the comment) so it's needed even if it's not
> nice to have it there. You could move it elsewhere but just deleting
> it would break something.
>
> >Again, thanks a lot for your comments. Please, try the patches, I
> >think now is better.
>
> I've tried them and it does not seem better to me. Actually, it got
> worse. Try to do the test sequence with xcalc. With current next
> branch if there's nothing in the config it has no app icon (uses the
> default icon) only the miniwindow has an icon. Then the icon from
> the cache is still saved and the Ignore option checked wrongly
> mismatching the config file. I gave up at this point.
>
> Regards,
> BALATON Zoltan
Hi Zoltan!
I changed the behavior for the winspector. Can you test it now?
More testers are welcome. If the patch is ok, I will write the commit info.
Cheers,
kix
--
||// //\\// Rodolfo "kix" Garcia
||\\// //\\ http://www.kix.es/
>From e69d230a8c1fdb2aa43fc0c765bfac3c85ab7fa6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?"Rodolfo=20Garc=C3=ADa=20Pe=C3=B1as=20(kix)"?= <k...@kix.es>
Date: Fri, 25 Jan 2013 00:59:08 +0100
Subject: [PATCH] New winspector behavior
For testing only.
---
src/winspector.c | 81 ++++++++++++++++++++++++++++++++----------------------
1 file changed, 48 insertions(+), 33 deletions(-)
diff --git a/src/winspector.c b/src/winspector.c
index 7335030..8d5923a 100644
--- a/src/winspector.c
+++ b/src/winspector.c
@@ -475,29 +475,23 @@ static void saveSettings(WMButton *button, InspectorPanel *panel)
winDic = WMCreatePLDictionary(NULL, NULL);
appDic = WMCreatePLDictionary(NULL, NULL);
- /* If the "Ignore client suplied icon is not selected" flag was not set,
- * then, don't save the icon filename. If saved, the application will use
- * that icon, even the flag is not set. */
- if (WMGetButtonSelected(panel->alwChk) != 0) {
- /* Update icon for window */
- icon_file = WMGetTextFieldText(panel->fileText);
- if (icon_file) {
- if (icon_file[0] != 0) {
- value = WMCreatePLString(icon_file);
- different |= insertAttribute(dict, winDic, AIcon, value, flags);
- different2 |= insertAttribute(dict, appDic, AIcon, value, flags);
- WMReleasePropList(value);
-
- /* Set the ckeck for AAlwaysUserIcon only if icon_file exists */
- buf1 = wmalloc(4);
- snprintf(buf1, 4, "%s", (WMGetButtonSelected(panel->alwChk) != 0) ? "Yes" : "No");
- value1 = WMCreatePLString(buf1);
- different |= insertAttribute(dict, winDic, AAlwaysUserIcon, value1, flags);
- WMReleasePropList(value1);
- wfree(buf1);
- }
- wfree(icon_file);
- }
+ /* Save the icon info */
+ /* The flag "Ignore client suplied icon is not selected" */
+ buf1 = wmalloc(4);
+ snprintf(buf1, 4, "%s", (WMGetButtonSelected(panel->alwChk) != 0) ? "Yes" : "No");
+ value1 = WMCreatePLString(buf1);
+ different |= insertAttribute(dict, winDic, AAlwaysUserIcon, value1, flags);
+ WMReleasePropList(value1);
+ wfree(buf1);
+
+ /* The icon filename (if exists) */
+ icon_file = WMGetTextFieldText(panel->fileText);
+ if ((icon_file) && (icon_file[0] != 0)) {
+ value = WMCreatePLString(icon_file);
+ different |= insertAttribute(dict, winDic, AIcon, value, flags);
+ different2 |= insertAttribute(dict, appDic, AIcon, value, flags);
+ WMReleasePropList(value);
+ wfree(icon_file);
}
i = WMGetPopUpButtonSelectedItem(panel->wsP) - 1;
@@ -746,24 +740,45 @@ static void applySettings(WMButton *button, InspectorPanel *panel)
file = NULL;
}
- /* If file is NULL, the always_user_icon doesn't matter,
- * because we need to read the icon from the window */
- if (file && WFLAGP(wwin, always_user_icon)) {
- /* Change icon image if the app is minimized */
- if (wwin->icon)
- wIconChangeImageFile(wwin->icon, file);
+ /* If always_user_icon flag is set, but the user icon is not set
+ * we use client supplied icon and we unset the flag */
+ if ((WFLAGP(wwin, always_user_icon) && (!file))) {
+ /* Show the warning */
+ char *buf;
+ int len = 100;
- /* Change App Icon image */
+ buf = wmalloc(len);
+ snprintf(buf, len, _("Ignore client supplied icon is set, but icon filename textbox is empty. Using client supplied icon"));
+ wMessageDialog(panel->frame->screen_ptr, _("Warning"), buf, _("OK"), NULL, NULL);
+ wfree(buf);
+ wfree(file);
+
+ /* Change the flags */
+ WSETUFLAG(wwin, always_user_icon, 0);
+ WMSetButtonSelected(panel->alwChk, 0);
+ }
+
+ /* After test the always_user_icon flag value before,
+ * the "else" block is used only if the flag is set and
+ * the icon text box has an icon path */
+ if (!WFLAGP(wwin, always_user_icon)) {
+ /* Change App Icon image, using the icon provided by the client */
if (wapp->app_icon)
- wIconChangeImageFile(wapp->app_icon->icon, file);
+ wIconUpdate(wapp->app_icon->icon,
+ get_rimage_icon_from_wm_hints(wapp->app_icon->icon));
+
+ /* Change icon image if the app is minimized,
+ * using the icon provided by the client */
+ if (wwin->icon)
+ wIconUpdate(wwin->icon, get_rimage_icon_from_wm_hints(wwin->icon));
} else {
/* Change App Icon image */
if (wapp->app_icon)
- wIconUpdate(wapp->app_icon->icon, get_rimage_icon_from_wm_hints(wapp->app_icon->icon));
+ wIconChangeImageFile(wapp->app_icon->icon, file);
/* Change icon image if the app is minimized */
if (wwin->icon)
- wIconUpdate(wwin->icon, get_rimage_icon_from_wm_hints(wwin->icon));
+ wIconChangeImageFile(wwin->icon, file);
}
if (file)
--
1.7.10.4
