I looked into this and implemented it. It assumes the existence of an icon called 'window', which is not in g-i-t- yet (Where can I contact the Art Team?). 'window-new' without the star would probably do it. I hope the patch/changelog fits the guidelines.

Regards,
Patrick

Created attachment 114629 [details] [review]
Default Window Icon From Stock Patch

Hi Patrick, great to have your input.  Seems like you're on the ball with the coding conventions and everything like that, and the patch seems to do what's wanted much more cleanly than before.  Here are a few points to consider, though:

1) Most seriously, it doesn't compile with a fresh checkout; src/ui/preview-widget.c needs the changes you've already made to src/ui/ui.c.  This should be almost identical (though possibly we should have a function somewhere to avoid the copy-and-paste to bring the four versions down to one, though I'm not sure it's necessary for such a short piece of code).  This will mean we have to introduce an include of gtk/gtkicontheme.h in src/ui/preview-widget.c, but I can't see why that would be a problem.

2) I would ordinarily have said not to put META_DEFAULT_ICON_NAME in common.h for reasons of information hiding, but (1) might make it necessary after all.

3) Personally I would have considered not having icon_exists and just doing the if on the result of the gtk_icon_theme_has_icon() call, but that's just me.  Feel free to ignore this.

4) It's best (at least with Metacity) not to put your ChangeLog entry in the patch, because patches get applied in unpredictable orders and it confuses the patch program.  Better to put it into a comment in the bug tracker.

Again, welcome and thanks!

1) I should probably check if it compiles *after* I edit the makefiles. Sorry for that.

Yes it would probably be better to have a new function like
GdkPixbuf* get_default_window_icon (gint size), but I'm not sure where to place this function. (src/ui/utils.c would be obvious, but there is no such file.)
Thus, I just copied the code into the corresponding functions in src/ui/preview-widget.c.

Btw, is it on purpose that there is no g_object_ref (G_OBJECT (default_icon)) in meta_preview_get_icon and meta_preview_get_mini_icon?

2) Still in common.h, see above.

4)

ChangeLog:

* src/ui/ui.c: Use GtkIconTheme to load the default window icon.
Assumes the existence of an icon called "window", otherwise
falls back to "gtk-missing-image". Fixes #524343.
* src/ui/preview-widget: See above.
* src/include/common.h: Add META_DEFAULT_ICON_NAME.
* src/Makefile.am: Remove default_icon.png from inlinepixbufs.h.
* src/default_icon.png: Removed.

Created attachment 114662 [details] [review]
Edited Patch

Goodness, I seem to have lost track of this bug for a month.  So sorry.

The new patch is good and I will commit it after we branch for 2.26.  However, the icon called "window" might need some thinking about: see

  http://standards.freedesktop.org/icon-naming-spec/icon-naming-spec-latest.html

Is there an existing one there we can use?  If not, that gives the names of the people to talk to, and it might be worth inviting them to join this discussion.

Absent any change or decision there, it might be better to go with "applications-other"-- what do you think?

>
> Goodness, I seem to have lost track of this bug for a month.  So sorry.
>

No problem.

>
> The new patch is good and I will commit it after we branch for 2.26.
> 

Nice. :)

>  
> http://standards.freedesktop.org/icon-naming-spec/icon-naming-spec-latest.html
> 
> Is there an existing one there we can use?  If not, that gives the names of the
> people to talk to, and it might be worth inviting them to join this discussion.
> 
>

I don't think so, we could maybe abuse an icon, but it would be cleaner to just add a default application icon.

>
> Absent any change or decision there, it might be better to go with
> "applications-other"-- what do you think?
> 

"applications-other" is a category icon intended for the usage in (gnome) menus. I think that would be confusing.

Maybe "application-default" would be a better name for the icon.

Regards,
Patrick

I committed your patch in trunk.  Thanks a lot:

http://svn.gnome.org/viewvc/metacity?rev=3812&view=rev

Can you get on to the fdo people and ask them about the new icon name?  I can probably help you out if you get stuck somewhere along the line.

http://live.gnome.org/ThemableAppSpecificIcons

This is probably the best course of action here, if there must be an icon. What about just not having an icon at all?

If you are looking for the correct solution according to the spec though, it is that applications provide icons matching their binary name. If they aren't, then icons should probably be drawn, and the applications fixed, as they aren't following the naming spec anyway.

It's not an option not to have an icon.  Several themes use the icon as something to click to get the window menu.  This would result in an application which didn't have an icon not having a window menu.

Patrick: how do you feel about the spec given above, and working on it?

Rodney: I can't resolve pronominal anaphora in your last sentence in comment 9.  Can you rephrase it?

Not having an icon wouldn't mean the menu wouldn't exist. The menu is still accessible, but there just wouldn't be an icon to click to get it. You can still right-click anywhere on any part of the window border, to access it. There is also a keybinding to access it, but I dont'r remember what that is right now.

My last sentence in my previous comment was to say that applications which do not have a window icon, are not following the specs anyway, in not providing the icons. If there are enough applications on a general system to warrant the need for this workaround in metacity, then I think it best to help fix those applications, rather than shove a workaround (or keep the one we have, just make it more "themable"), into metacity, and just ignore those applications.

THe main menu in gnome-panel is already getting it wrong anyway, because it places a generic window icon for desktop files where the Icon they specify is not found. It doesn't matter if the resulting window upon launch actually has an icon or not. For example, on Ubuntu, you can install Beneath A Steel Sky, which specifies an icon in the .desktop file, but does not provide that icon. The launcher in the main menu shows a generic "window" icon, because it can't find the icon. If I drag that launcher to the panel, it then gets an image-missing icon, and in the wrong size for some reason. If I then drag the same launcher to my desktop, it gets yet another icon, this time the really old, ugly classic paper sheet icon from GNOME 1.x days... However, launching the program, the window icon has the SCUMMVM icon, as it's just running inside that virtual machine.

Clearly there is a severe lack of consistency here, that extends far beyond metacity. :)

The keybinding is alt-space (by default), but I'm still not very happy about the icon disappearing; right-click on titlebar isn't very discoverable.

Applications which don't provide icons may be broken, but half of the code in Metacity is there to deal with broken applications.  (After all, if we could assume that all the websites in the world produced nothing but valid XHTML, I could write a browser in a week, but as we know they don't and that's why Mozilla took four years to release.)

What's the "generic application icon" that the panel uses for .desktop files which don't have a valid icon?

You give an interesting example, and I think we should discuss it, perhaps on planet or d-d-l.

> Clearly there is a severe lack of consistency here, that extends far beyond
> metacity. :)

Well, that should not keep us from fixing the main regression here, which is that windows now show up with a missing-image icon. just add that icon to metacity and be done with it. Fighting for standard icon names is a lost cause.

(In reply to comment #13)
> > Clearly there is a severe lack of consistency here, that extends far beyond
> > metacity. :)
> 
> Well, that should not keep us from fixing the main regression here, which is
> that windows now show up with a missing-image icon. just add that icon to
> metacity and be done with it. Fighting for standard icon names is a lost cause.
> 

I think you commented on the wrong bug, and please stop being rude with your misplaced anger against me.

The change makes metacity inconsistent with window-list in gnome-panel (libwnck).

The default window icon has been included in bug 616246