Code review comment for lp://qastaging/~dandrader/qtmir/keyState

> On 20/03/2017 14:08, Gerry Boland wrote:
> > Review: Needs Information
> >
> > If the surface looses active focus, don't we inform Mir?
>
> It's the other way around. Mir surface focus is what drives qml active
> focus. Nowadays qml active focus by itself doesn't carry much meaning in
> terms of window management.

There are more things in the QML scene that just MirSurfaces that can get activeFocus though. When activeFocus does switch to the search box in the AppDrawer (as example), we are indeed telling Mir somehow that the MirSurface has not got input focus (I just tested gedit, the cursor does stop flashing when the text box in the app drawer is focused). Since this bug occurs due to focus changing while modifier key is down, my initial thinking was we could use the focus change to implement a workaround.

What I don't know is if the WMpolicy if notified when the MirSurface it has selected to be focused has lost actual input focus (activeFocus). Do you know?

> > Why not send the key release event then? Would be more efficient than
> checking on every key_up
>
> Because the key is actually still pressed. If the same window gets focus
> again moments later and only then the key is lifted the client would
> receive a second key_up event. Unless we keep track of what lies we have
> told already to the clients or devise some other special rules.

This appears to be a QML subtlety we're working around, as a non-QML Qt shell would not suffer from this issue, right? So could the MirSurfaceItem keep track of the key state itself? If the MSI looses activeFocus, it send key-release to the client. Then if it gets focus again, check if key is still down (not sure how tbh, maybe QML sends the key to the Item again immediately??), and if so, re-send it to client.

I'm not 100% certain what is the best approach yet.

> > Also, please avoid using singleton pattern, it makes testing impossible.
>
> Yeah, that was the easiest (laziest) way to get the two talking (event
> feeder and policy). Will see if I can get something nicer. Suggestions?

Nothing better than the usual piping an object around, so everyone who needs it gets a pointer to it. Sorry :(

> But before I spend too much time on the implementation details of this
> MP I would like to get feedback on the overall approach first.

Ah ok, I hope I've supplied that at least

>
> > And speaking of which, can you add test?
>
> For WindowManagementPolicy?

More the logic you're adding. But please ignore me until you settle on the approach you want.