After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 98340 - no border feedback on alt-tab with borderless themes
no border feedback on alt-tab with borderless themes
Status: RESOLVED FIXED
Product: metacity
Classification: Other
Component: general
2.12.x
Other Linux
: High minor
: 2.14.x
Assigned To: Metacity maintainers list
Metacity maintainers list
: 318503 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2002-11-12 20:48 UTC by Bill Nottingham
Modified: 2006-01-20 21:49 UTC
See Also:
GNOME target: ---
GNOME version: 2.13/2.14


Attachments
make outline width constant (2.42 KB, patch)
2005-10-08 02:42 UTC, Björn Lindqvist
needs-work Details | Review
Fixes 98340, fixes border feedback on hidden windows, fixes showing-desktop miss (2.52 KB, patch)
2005-11-21 16:44 UTC, Björn Lindqvist
needs-work Details | Review
Modify Bjorn's patch to match Havoc's per-side checking tweak in comment 17 (3.32 KB, patch)
2006-01-16 17:27 UTC, Elijah Newren
committed Details | Review

Description Bill Nottingham 2002-11-12 20:48:27 UTC
When using alt-tab to cycle windows, you get the border feedback on normal
windows, but not on maximized windows.
Comment 1 Heath Harrelson 2002-11-12 20:57:24 UTC
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?
Comment 2 Bill Nottingham 2002-11-12 20:58:51 UTC
2.4.0.92.
Comment 3 Havoc Pennington 2002-11-12 20:59:32 UTC
conceivably only broken for themes that are borderless when maximized. 

relevant code hasn't changed since the version I know bill has. ;-)
Comment 4 Bill Nottingham 2002-11-12 21:01:30 UTC
Yeah, if I switch to 'Bright' (for example) it works.
Comment 5 Havoc Pennington 2003-02-25 20:47:20 UTC
Confirmed for Bluecurve
Comment 6 Rob Adams 2003-02-28 20:02:28 UTC
presumably fixing Bug 97703 will break this for all themes.
Comment 7 Elijah Newren 2005-10-07 16:08:33 UTC
Remove old target milestones on old bugs; sorry for the spam.
Comment 8 Björn Lindqvist 2005-10-08 02:41:15 UTC
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.  
Comment 9 Björn Lindqvist 2005-10-08 02:42:48 UTC
Created attachment 53216 [details] [review]
make outline width constant
Comment 10 Elijah Newren 2005-10-08 21:21:27 UTC
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?
Comment 11 Björn Lindqvist 2005-10-09 21:10:46 UTC
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?
Comment 12 Elijah Newren 2005-10-09 21:30:37 UTC
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.  :)
Comment 13 Elijah Newren 2005-10-10 20:39:17 UTC
*** Bug 318503 has been marked as a duplicate of this bug. ***
Comment 14 Havoc Pennington 2005-10-21 02:17:12 UTC
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 ;-)
Comment 15 Björn Lindqvist 2005-10-26 03:22:07 UTC
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. :)  
Comment 16 Elijah Newren 2005-11-19 22:28:39 UTC
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.
Comment 17 Havoc Pennington 2005-11-19 23:50:11 UTC
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)
Comment 18 Elijah Newren 2005-11-19 23:53:53 UTC
Aidan fixed the minimized flag thing in bug 168455, which is probably what
causes this patch to no longer apply...
Comment 19 Björn Lindqvist 2005-11-21 16:44:22 UTC
Created attachment 55031 [details] [review]
Fixes 98340, fixes border feedback on hidden windows, fixes showing-desktop miss
Comment 20 Björn Lindqvist 2005-11-21 16:54:55 UTC
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.

Comment 21 Elijah Newren 2005-12-27 20:23:42 UTC
> 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?
Comment 22 Havoc Pennington 2005-12-27 22:03:34 UTC
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?
Comment 23 Elijah Newren 2005-12-27 23:47:21 UTC
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.
Comment 24 Elijah Newren 2006-01-14 21:47:49 UTC
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!
Comment 25 Björn Lindqvist 2006-01-16 09:32:57 UTC
I don't think I will have time. So if someone else wanna do it, it would be good.
Comment 26 Elijah Newren 2006-01-16 17:27:33 UTC
Created attachment 57483 [details] [review]
Modify Bjorn's patch to match Havoc's per-side checking tweak in comment 17
Comment 27 Elijah Newren 2006-01-20 21:49:15 UTC
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.