Created attachment 56944 [details] [review]
Add an orthogonal raise option

My quite possibly faulty memory of my Red Hat patch is that it only affected sloppy/mouse focus, which is a small grace.

I would not name the pref orthogonal_raise, just "raise_on_click" would be easier to find, no?

I still think this one is pretty detrimental to any ambitions GNOME may have to be a good desktop, but it's required for the UNIX and tech workstation market. The two are just in conflict. The tech-user-oriented distributions (i.e. all of them) (and where "the distributions" included me in my role as RHEL maintainer) are obviously vetoing any decision we might make, so it's not like it makes any difference.

Anyway, may as well make life easy for the package maintainers. I consider this one to be a big "we suck and show no signs of stopping" signal though, since nobody has shown any interest in fixing it properly.
</whine>

Created attachment 57119 [details] [review]
s/orthogonal_raise/raise_on_click/ (and reverse logic) and ignore option in click-to-focus-mode

Ok, here's the patch that makes the changes you requested.

Committed now.

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

I have an objection to what I believe is this section of the patch:

gboolean
meta_prefs_get_raise_on_click (void)
{
  /* Force raise_on_click on for click-to-focus, as requested by Havoc
   * in #326156.
   */
  return raise_on_click || focus_mode == META_FOCUS_MODE_CLICK;
}

This makes it impossible to control raise_on_click for my preferred META_FOCUS_MODE_CLICK. Should this not be left to the user to decide?

I also don't see Havoc actually having requested this particular part of the patch in his comments above.

Trying to build now with

gboolean
meta_prefs_get_raise_on_click (void)
{  return raise_on_click;
}

and see if it behaves like my (presumably Debian-patched) 2.12.3 where clicking on border works to raise.

Well, this build thing turned out to be simpler than I feared from seeing GARNOME.
I'm happy to say that with this patch Metacity behaves even better than expected:

Settings are: auto_raise=false focus_mode="click" raise_on_click=false

 - click to focus works without raising the windows.
 - windows can be raised by clicking on their border
 - windows can be moved or resized without raising them

Please consider this change for inclusion.

Created attachment 65906 [details] [review]
More complete patch for raise behaviour change

This patch includes the one-line change and adds a raise_on_activate gconf option to control the raising of windows when raise_on_click=false and a window is selected from the Window List panel application (among others).

(In reply to comment #6)
> I also don't see Havoc actually having requested this particular part of the
> patch in his comments above.

You'll need to re-read his comments then:

  "My quite possibly faulty memory of my Red Hat patch is that it only affected
  sloppy/mouse focus, which is a small grace."

(In reply to comment #7)
> Well, this build thing turned out to be simpler than I feared from seeing
> GARNOME. I'm happy to say that with this patch Metacity behaves even better
> than expected:

Yes, the former debian and RHEL patches were just hacks (as were all patches that existed in various bugs in bugzilla).  And yes, your change is correct and won't have any nasty side effects for click-to-focus (and was, in fact, the original version).  I'm curious, though, what was it that you saw in GARNOME?  Do they have some patch in there related to this feature?

> Please consider this change for inclusion.

I intend to eventually include this change, but not until DND is fixed so that we have the majority use case right (i.e. so that this new unique mode is used correctly by the minority that want it, instead of it mostly being utilized for a cop-out workaround to those DND bugs by the majority who otherwise don't want this feature).  See e.g. bug 76672, bug 80984, and bug 152952 among others for more details.  If you want to ask seb128 to include this modification in debian/ubuntu patches, feel free.  (Though, the return statement ought to be on its own line to match the style of the surrounding code...)

(In reply to comment #8)
> Created an attachment (id=65906) [edit]
> More complete patch for raise behaviour change
> 
> This patch includes the one-line change and adds a raise_on_activate gconf
> option to control the raising of windows when raise_on_click=false and a
> window is selected from the Window List panel application (among others).

Waaay to nit-picky and I don't see how this is at all useful.

> Waaay to nit-picky and I don't see how this is at all useful.

With raise_on_click=false it is sometimes difficult to bring a window to the foreground, as it needs a click on the window border. The window list panel applet seems the ideal choice to be able to raise windows too.
Adding the raise_on_activate option is a necessary evil because otherwise the behaviour would change for mouse/sloppy focus modes too.

Now, my understanding of this is still pretty weak, so maybe my patch is wrong. I notice clicking a window in the window list panel applet causes an activation event, on which I want to raise the window. Are there more events that cause activation events? How can I find out?

I realize you'd prefer there not to be a proliferation of configuration options, especially if they are in the configuration dialogs. However, this option (if it is correct -- I'm happy to rework the patch it if not) is a pretty minor one, and could be just a gconf-editor or gtweakui configurable, that would suffice.

Maybe you'd prefer this as a focus mode instead of separate booleans (with not all selectable combinations making sense)?

(In reply to comment #10)
> > Waaay to nit-picky and I don't see how this is at all useful.
> 
> With raise_on_click=false it is sometimes difficult to bring a window to the
> foreground, as it needs a click on the window border. The window list panel
> applet seems the ideal choice to be able to raise windows too.

Agreed, clicks on the window list should focus and raise the relevant window.  That's why the code does that.  Why add a configuration option to allow someone to configure the window list to be able to do what it already does?  That makes no sense.  However, there was a bug in an old version of libwnck that would cause it to not get raised, but that was just a bug.  Just update your copy of libwnck if you're affected by it.

> Adding the raise_on_activate option is a necessary evil because otherwise the
> behaviour would change for mouse/sloppy focus modes too.

I don't understand this at all.

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

I prefer sloppy focus mode anyway, so it's not all that important to me, but I don't understand the rationale for overriding raise_on_click in click-to-focus mode. It defaults to on anyway, so honouring the option won't catch out new users.

raise_on_click may make it easier to raise a window when you can't see its borders, but there are modifiers etc to overcome that. OTOH, if you're stuck with a small screen eg on a laptop, so that two filer windows won't fit without overlapping, raise_on_click makes actions like copying/moving files from one window much, much harder. If you want to make a great desktop, taking away choice and aping Windows right down to the features, or lack of, that make its desktop awful, isn't the way to do it.

Created attachment 96573 [details] [review]
No automatic raise_on_click for click_to_focus

This patch removes the automatic raise_on_click for click_to_focus which makes it possible to click on a window to focus (or move) it without bringing it to foreground.

> I don't understand the rationale for overriding raise_on_click in
> click-to-focus mode.

The rationale is essentially that people are abusing it as a workaround to a different bug:

> raise_on_click makes actions like copying/moving files from one
> window much, much harder.

The click of a DND shouldn't cause a raise regardless of the setting of this flag.  See bug 152952 and bug 154260.

I think we must be misunderstanding each other about what we want the click_to_raise option to do. But I doubt anybody's enirely happy with the behaviour when click_to_raise=0. I just want an option that prevents windows being raised when I click inside them but disabling raise_on_click has other side-effects that, judging by the gconf comment, the developers consider broken as well as users like me. So why not get rid of these side-effects? It makes the code simpler and makes the option name a more accurate description. There's also no reason to override raise_on_click when focus_mode=click.

I'm attaching a patch.

Created attachment 98981 [details] [review]
Changes behaviour when setting raise_on_click=0

I'm the author of the original feature.  The gconf key is misnamed at the previous maintainer's request, so if anything the key should be renamed.  I agree with you about the focus_mode=click part of things in the long run, but think we should fix raise with DND actions for all users first to prevent further abuse of this feature (by the way, the disabling of this feature in click to focus was also a request by Havoc).  Anyway, the main reason I say the key is broken is that too many people use it as a workaround for the don't-raise-on-DND issue (and then complain about other aspects of the actual feature this was designed for).  The other issue is that we don't have enough information to do things completely perfectly, but requiring app authors to be aware of yet another state and providing even more information is entirely unreasonable.  The current behavior (minus perhaps the click-to-focus bits) is nearest the design considerations.  Modifying it as in your patch would be bad; the point of !raise_on_click is that it's easy to alt-click or alt-tab to raise the window, but a royal pain for the user to fix the stacking order when it gets messed as some strange side-effect of an unrelated request (e.g. focusing, resizing, moving).

This is insane. Havoc's comment could be interpreted as a request for disabling it in focus_mode=click, but all I see is a rant aimed at breaking the option as an excuse to drop the option in his quest to make Gnome dumber than Windows.

Then you say this bug was created deliberately to stop people working around another bug which you can't/won't fix. Don't you think that's a bit unreasonable?

Why do you think authors have to be aware of another state if you reduce the side effects from this option? In fact they have to be aware of extra issues because of the current behaviour (WNCK_CLIENT_TYPE_PAGER etc).

Either the stacking order getting "messed up" by windows being raised on activation etc is broken or it isn't, surely. If you think that's broken why only "fix" it for users who disable an option you don't want them to?

(In reply to comment #19)
> Then you say this bug was created deliberately to stop people working around
> another bug which you can't/won't fix. Don't you think that's a bit
> unreasonable?

That's not a fair characterization at all.  I've put work into fixing the other bug(s); just haven't had time to complete it.  These days, I spend near 100% of my available time that isn't spent on the release-team responding to bug reports.

> Why do you think authors have to be aware of another state if you reduce the
> side effects from this option? In fact they have to be aware of extra issues
> because of the current behaviour (WNCK_CLIENT_TYPE_PAGER etc).

Okay, I should have been more careful with my wording.  Pagers are very few in number compared to normal apps.  Only pager applications need fixing (to advertise that they really are a pager); which is something they should be doing anyway.
 
> Either the stacking order getting "messed up" by windows being raised on
> activation etc is broken or it isn't, surely. If you think that's broken why
> only "fix" it for users who disable an option you don't want them to?

The option is whether users like windows being raised as a side effect of other options (e.g. focusing, resizing, moving).  Thus, having the stacking order be modified is correct when the option is set to false, and broken when the option is set to true (except in the case of pagers, like the tasklist, because they know enough about global window state to intelligently muck with things like stacking; apps don't).  This is entirely intentional.  The current behavior regarding _NET_ACTIVE_WINDOW messages is not broken for either of the potential values of this gconf option.  For a short while, the libwnck applets were broken because they were very much a pager program but didn't register as such.  Perhaps there are other programs in the same boat, in which case there is breakage, it's just not in metacity.

Now, I'll admit I've taken pains to grudgingly agreeing that this option is "broken", though I don't really believe it.  The reasons were (1) we have too many ridiculous configuration options in metacity, (2) people want a trillion and one variants of each little option (you being just one of them), (3) I knew it'd be used as a workaround to another bug and I should have fixed the other bug *first*, then added this feature.  Shame on me.

Anyway, this feature was built on the following rationale:
  The point of !raise_on_click is to make it easy to keep the stacking order
  while doing other operations.  It is very easy for the user to manually raise
  the window as a separate action, but difficult to restack several windows if
  the user is interested in a certain stacking.  Thus, we should avoid
  restacking unless we know for certain it was a user request to specifically
  restack.

I have !raise_on_click in use all the time, except sometimes when trying to reproduce other people's bugs.  It works perfectly.  That's probably not what you wanted to hear from me, but that's where I stand.

> Okay, I should have been more careful with my wording.  Pagers are very few in
> number compared to normal apps.  Only pager applications need fixing (to
> advertise that they really are a pager); which is something they should be
> doing anyway.

At the moment python libwnck clients (I use one) can't because of bug 483529 and it doesn't look like it's going to be fixed in a hurry.

> The option is whether users like windows being raised as a side effect of other
> options (e.g. focusing, resizing, moving).

I think there's another important issue of whether people want it to be possible to work in a window without it being on top of the stack, which isn't quite the same thing as whether application actions as opposed to user actions can cause raising. And the set of users 

> I have !raise_on_click in use all the time, except sometimes when trying to
> reproduce other people's bugs.  It works perfectly.  That's probably not what
> you wanted to hear from me, but that's where I stand.

The gconf schema comment makes it clear that at least one other developer considers that !raise_on_click causes breakage - in metacity.

I think if the current behaviour must be kept it would be better with the sense of the option reversed, as it was with "orthogonal_raise", but perhaps a name like "restricted_raise" would describe it better.

Is there any chance it could be replaced by an option with 3 values? The option is hidden anyway so there's no objection on grounds of making the capplet more complicated. The complication it would add to the code is quite insignificant because it just means changing a few if (meta_prefs_get_raise_on_click()) statements to if (meta_prefs_get_raise_policy() == SOME_CONSTANT).

I wrote the schema too (three different versions now, I think).  Long story.

Anyway, on the way to work I thought of something else that may be enlightening here, suggesting there's a further dimension to this issue.  Can you describe the exact use cases where you dislike what happens with the app window (the exact application you are using, the exact steps you take, perhaps a bit about the app)?  I'm wondering if my idea may be relevant, or if I'm just going crazy.  :-)

I can't really remember any specific cases now, but I have noticed actions that I'd expect to make a window appear apparently not do anything because the window is completely obscured behind others. One application that this can happen in is my own ROXTerm; if you try to open one of the configuration windows and it's already open it uses gtk_window_present() so if it's hidden behind others you wouldn't notice it. I think applications which hide windows when closed and reuse them later, rather than destroy and recreate them, might also run into this problem?

In reply to comment 22:
 - application windows to be brought to the foreground
   when selected by clicking inside
 - application windows to be brought to the foreground 
   when moved
 - application windows to be brought to the foreground 
   when clicking inside an already active window

I've created a patch. I don't realistically expect it to be accepted as is, but I hope it will move the discussion on a bit, because I don't think we've (Elijah and I) understood what each other wanted very well so far. Basically, this bug makes the difference as to whether I can use metacity. I use openbox instead now, but I'd rather use a "fixed" metacity. If bug 358674 was fixed too it would be just about perfect.

It replaces the raise_on_click option with raise_policy, taking 3 possible values: "always", "unix" and "decoupled" (difficult to think of good names). "decoupled" is similar to raise_on_click=0 but it isn't overridden by focus-mode=click (more on that later). "always" is similar to raise_on_click=1 and "unix" is somewhere in between: users can click in client areas and drag the frame without raising a window, but application requests to raise a window are honoured.

Reading the comments around the code I changed, I did get some more insight into why raise_on_click is like it is, but I'm still puzzled by some things. ISTM as a developer Elijah is not looking so much at what stands out to me as a user, but is concentrating on an abstract concept ie making the stacking order consistent with MRU. But why is that important, and isn't it impossible to achieve anyway as long as users can lower windows by middle-clicking the title bar?

As to why raise_on_click was forced on when focus_mode=click. The only reason I've seen for that so far is because Havoc said it was a saving grace (comment 9), but I'm having trouble finding justification for why it's a saving grace. Is it to do with bug 102209 and 115072?

I don't think I need to justify that there's a demand for !raise-on-click, but 
with regards to comment 22 again, would you like a test case to show what I don't like about raise_on_click=0 compared with raise_policy=unix?

Created attachment 103453 [details] [review]
Replaces raise_on_click boolean option with raise_policy (three possible string values)

Created attachment 103645 [details] [review]
Replaces raise_on_click boolean option with raise_policy (three possible string values)

I found there were still some quirks with metacity opening new windows behind existing ones (eg claws-mail's compose dialog) so I've modified my patch.

I can't seem to get this to work the way I want. I think we don't actually disagree after all on how we want !raise_on_click to work, but that it's currently bugged. Please look at bug 513281.