Created attachment 261250 [details] [review]
display: Fix logic for determining whether our focus was successful

In some cases, we can focus the frame window instead of the client
window, so make sure that our checks include that as well.

Created attachment 261251 [details] [review]
display: Use MetaWindow for auto-raise callbacks

This allows us to autoraise Wayland windows... well, except for the
XQueryPointer, but we'll replace that soon.

Created attachment 261252 [details] [review]
Move autoraise callbacks into window.c

This matches what the wayland branch does, which allows us to share
code more easily.

Created attachment 261253 [details] [review]
window: Reword a condition to make it easier to understand

Created attachment 261254 [details] [review]
window: Separate pointer-focus operations and explicit-focus operations

Some clients, like on-screen keyboards, don't want their windows to be
activated on click, as that would steal focus from the window they're
trying to send events to. They do this by setting the Input Hint in
WM_HINTS to be FALSE, along with not specifying WM_TAKE_FOCUS in their
WM_PROTOCOLS.

However, in this case, when a window tries to get focus in this scenario,
we focus the frame so that a11y and key navigation works properly.
Both policies aren't acceptable -- we can't make OSK steal focus from
windows on click, and we can't make an OSK un-Alt-Tabbable-to.

To solve this, split meta_window_focus() into two different focus policies:

 * meta_window_focus_implicitly() should be called on pointer click or
   focus-follows-mouse or similar cases where we may not want to forcibly
   set focus for clients that don't want it.

 * meta_window_focus_explicitly() should be called by pagers, like
   the gnome-shell overview, or Alt-Tab. In this case, most of the existing
   clients are using meta_window_activate(), so simply adapting that so
   it calls meta_window_focus_explicitly() should be enough.

Review of attachment 261250 [details] [review]:

Looks right

Review of attachment 261251 [details] [review]:

Looks good.

Review of attachment 261252 [details] [review]:

Mostly seems like fine code movement. I'll take your word that it matches the wayland branch better. One piece seems misplaced.

::: src/core/window.c
@@ +11436,3 @@
+
+static void
+reset_ignored_crossing_serials (MetaDisplay *display)

Think this should have stayed in MetaDisplay, since all the other handling of this is in MetaDisplay - I don't mind a method in window.c that operates on MetaDisplay *if* it is closely related to other code that lives in window.c, but this doesn't seem to to be the case here.

Review of attachment 261252 [details] [review]:

Mostly seems like fine code movement. I'll take your word that it matches the wayland branch better. One piece seems misplaced.

::: src/core/window.c
@@ +11436,3 @@
+
+static void
+reset_ignored_crossing_serials (MetaDisplay *display)

Think this should have stayed in MetaDisplay, since all the other handling of this is in MetaDisplay - I don't mind a method in window.c that operates on MetaDisplay *if* it is closely related to other code that lives in window.c, but this doesn't seem to to be the case here.

Review of attachment 261253 [details] [review]:

In English I'd express the condition as "the window does not have the input hint or take focus". You've rewritten it to "the window neither has the input hint nor does it take focus".

Which is clearer is probably a matter of the reader.

Review of attachment 261254 [details] [review]:

The idea that meta_window_focus_implicitly() and meta_window_focus_explicitly() are two equal variants doesn't seem right to me. If you look through the code in Mutter, you notice that in almost all the cases where we call meta_window_focus() at the moment the code would just be broken if we didn't focus the window.

As a random example, in workspace.c:focus_ancestor_or_top_window() the code decides on a window and then calls meta_window_focus() on it. If the code decided not to focus the window, the focus would end up left in a random, unintended place.

The idea that there's some variant of meta_window_focus() that can be called and might simply do nothing if the window was not suitable for focusing only makes sense in very limited cases that the caller has to be aware of.

To me, that's really only two cases:

 - Clicking on the client window, not the frame. (If you click on the frame, I think it's very odd if the window frame stays in an inactive state. Generally OSK type windows won't have a titlebar.)
 - Entering a window with mouse/sloppy focus.

I think the clearest thing to do - to make it clear in the calling places what's going on would be to simply insert in those very few places:

 if (meta_window_wants_key_input (window))
   meta_window_focus();

This also leaves it up to the caller if "other things" that the caller is doing are done or not - in particular, should we be auto-raising such windows? (I don't know the answer, but I do know that the code will make it obvious what's going on in that respect  if the condition is in the caller.)

[ Alternate approach would be to have a 'gboolean meta_window_maybe_focus_for_mouse_action(MetaWindow *window)' - but I think the above is more flexible. ]

Review of attachment 261254 [details] [review]:

The idea that meta_window_focus_implicitly() and meta_window_focus_explicitly() are two equal variants doesn't seem right to me. If you look through the code in Mutter, you notice that in almost all the cases where we call meta_window_focus() at the moment the code would just be broken if we didn't focus the window.

As a random example, in workspace.c:focus_ancestor_or_top_window() the code decides on a window and then calls meta_window_focus() on it. If the code decided not to focus the window, the focus would end up left in a random, unintended place.

The idea that there's some variant of meta_window_focus() that can be called and might simply do nothing if the window was not suitable for focusing only makes sense in very limited cases that the caller has to be aware of.

To me, that's really only two cases:

 - Clicking on the client window, not the frame. (If you click on the frame, I think it's very odd if the window frame stays in an inactive state. Generally OSK type windows won't have a titlebar.)
 - Entering a window with mouse/sloppy focus.

I think the clearest thing to do - to make it clear in the calling places what's going on would be to simply insert in those very few places:

 if (meta_window_wants_key_input (window))
   meta_window_focus();

This also leaves it up to the caller if "other things" that the caller is doing are done or not - in particular, should we be auto-raising such windows? (I don't know the answer, but I do know that the code will make it obvious what's going on in that respect  if the condition is in the caller.)

[ Alternate approach would be to have a 'gboolean meta_window_maybe_focus_for_mouse_action(MetaWindow *window)' - but I think the above is more flexible. ]

Attachment 261250 [details] pushed as f0bc53c - display: Fix logic for determining whether our focus was successful
Attachment 261251 [details] pushed as ce3804e - display: Use MetaWindow for auto-raise callbacks

Jasper, when this is done, can you backport it to 3.10, so we can have working onscreen keyboards in f20 ?

We've reverted the gtk change in 3.10 and master, too.

So I think this can be closed

waiting for working gtk3 fedora build to see what latest mutter and gtk3 package gives result for this OSK bug

I think there is no mutter release for 3.10.x that included patch from https://bugzilla.gnome.org/show_bug.cgi?id=710296#c6 ,so the OSK bug still exists in Fedora 20 Final TC5 with gtk3-3.10.6-1.fc20 updated