If by "border feedback" you mean "Metacity draws a black-and-white box
around the window", it's working here with the November 7 Ximian
snapshot from CVS.

Which version of Metacity do you have?

2.4.0.92.

conceivably only broken for themes that are borderless when maximized. 

relevant code hasn't changed since the version I know bill has. ;-)

Yeah, if I switch to 'Bright' (for example) it works.

Confirmed for Bluecurve

presumably fixing Bug 97703 will break this for all themes.

Remove old target milestones on old bugs; sorry for the spam.

There is a simple fix for this - set the outline bordersize to a constant (e.g.
5) instead of it being dependant on the theme's bordersize. That gives two
advantages: the border is always shown around the window wheter it is maximized
or not and it is also shown correctly in the taskbar when the application is
minimized (which it isn't right now). I'm attaching a patch that fixes this and
  an issue with the borders in showing desktop mode.  

Created attachment 53216 [details] [review]
make outline width constant

Works for me and I don't see any issues with the patch (well, other than the
fact that we still need to rename entries.minimized to something like
entries.not_showing or entries.hidden, which we are already keeping bug 168455
open about).  Havoc was there a reason you wanted to set up the border different
for framed and unframed windows?  I kind of like the idea of having the outline
be the same width for all windows as Björn suggests, but maybe I'm not aware of
some historical context?

I thought about renaming entries.minimized to entries.not_showing but I didn't
want the patch to contain non-essential changes too. There are some other
changes that would be good if this patch is a good idea. The x, y, width, height
fields in MetaTabEntry should be a MetaRectangle instead and the inner stuff
wouldn't be needed (since you can calculate that by subtracting 5) and the
MetaTabEntry and TabEntry structs could be merged since they are very similar to
each other. Would these changes be better handled in a new patch or would you
like me to add them to this patch?

Different issues are usually best placed in separate patches.  In this case, we
were already leaving bug 168455 open for the renaming of entries.minimized
(since it was very closely related to the original bug report), and I think bug
166980 is for the cleanup of MetaTabEntry vs. TabEntry.  I'm not aware of any
bug for the replacement with a MetaRectangle (note though that this isn't the
only place where four fields would be nice to replace with a MetaRectangle, as
I've noticed on the constraints_experiments branch), but it should also probably
be a separate bug.

Anyway, I'm going to reassign this bug's component since your patch neatly fixes
this bug in a theme-independent way.  :)

*** Bug 318503 has been marked as a duplicate of this bug. ***

This is basically an aesthetic thing, it looked nicer to me to have the outline
match the frame width. A simple alternate patch might be:

- if (window->frame)
+ if (!meta_rectangle_equals(&inner_rect, &outer_rect))

(fixing the naming of stuff to match reality)

And yes, please don't put in a comment about how entries.minimized is different
from window.minimized, fix the underlying situation ;-)

The problem is that the code should only create a border based on the frame
geometry if the window is visible and not maximized. When the window is
maximized, most themes do not paing east, west and south borders - only the
titlebar. So the black border highlighting will not be visible. When the window
is minimized, you can't rely on window->frame.rect's values to calculate the
position of the inner_rect. The current border feedback you get is buggy
although there is no open bug about it. Also, the mist theme only draws north
and south borders so it looks  a little weird when you alt-tab (you get only two
black horizontal lines, no rectangle). IMHO this is a case where Metacity is
trying to be too clever for its own good. :)  

I don't see how Havoc's suggestion would fix the problem as maximized windows
still have a titlebar.  Could you clarify, Havoc?

Also, the patch no longer applies and needs to be updated.

I was thinking entirely borderless windows, but there's an obvious tweak to my
suggestion ... per-side use a fixed width if the side is 0-size and the actual
size of the side otherwise, is the most sophisticated tweak, "ignore the north
side" is a a simpler tweak.

I don't see why doing this nicely is "too clever" (other than taking a small
number of additional lines of code) - I remember it looking unpolished if you
didn't match the frame width and it's not adding anything especially complex or
error-prone if we fix it nicely.

Don't forget also the point about the minimized flag (rename it or fix it, don't
comment it)

Aidan fixed the minimized flag thing in bug 168455, which is probably what
causes this patch to no longer apply...

Created attachment 55031 [details] [review]
Fixes 98340, fixes border feedback on hidden windows, fixes showing-desktop miss

How bout this patch? It makes it so the whole window frame is included in the
border feedback. I think it looks cool and it fixes the problem with no border
feedback on maximized windows - the titlebar is always shown and becomes black.
It also fixes the bug with the border-feedback on iconified windows and the bug
in which the outer rect of the window was used instead of the iconified rect
when the desktop was in showing desktop mode. 

The reason why I thought that doing this nicely was too clever is because it
only made sense to calculate the outline if the window had a frame, was not
maximized (since maximized windows often doesn't have east, west or south
borders) and not iconified/hidden. I may be wrong (because almost all my windows
are maximized), but I think differences in border feedback on
maximized/non-maximized windows is not so nice IMHO.

> I think differences in border feedback on maximized/non-maximized windows is
> not so nice IMHO.

Havoc's suggestion (for each side, if there is a frame and the width of the frame on that side is nonzero then use it, otherwise use a thickness of 5), modified to handle the hidden case as your patch does (thanks for fixing that too!), would also remove the differences in border feedback between the window types.  So, it could just as easily be seen as not being quite clever enough rather than being too clever.  ;-)

Your patch works fine for me and I don't see any technical problems with it, though Havoc would need to weigh in on whether he'd want to make the black-out-the-whole-frame change.  Any chance you could also code up his suggestion and then Havoc could just pick one of your two patches?

Don't you guys think blacking out the whole titlebar is kinda ugly? (OK, granted, all of the options here look sort of asstastic)

I can imagine it's nice to be able to at least sort of see the window title in this context though?

Well, I was pretty apathetic about the visual changes after trying it, so I just didn't bother commenting on them previously.  But, I hadn't thought about alt-esc until your comments, and I agree that it'd be a regression to not be able to see the window title when using that.  (Now, that regression won't actually exist until bug 325092 is fixed, but it will be a problem.)  I'm updating the patch accordingly.

Björn: I'd really like to get a version of your patch fixed up according to Havoc's comments in before UI freeze, which leaves us about two weeks.  Will you have time for that, or will you need someone else to make the necessary tweaks?
Thanks for all your help!

I don't think I will have time. So if someone else wanna do it, it would be good.

Created attachment 57483 [details] [review]
Modify Bjorn's patch to match Havoc's per-side checking tweak in comment 17

2006-01-20  Elijah Newren  <newren gmail com>

	Patch from Björn Lindqvist to fix #98340.

	* src/screen.c (meta_screen_ensure_tab_popup): Make sure an
	outline border is shown even if a window frame's width is 0.
	Also, correctly handle window outlines in showing desktop mode.