Mir

[regression] OSK focus issues

Bug #1289058 reported by kevin gunn on 2014-03-06
12
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Mir
Invalid
Critical
Unassigned
unity-mir
Critical
Gerry Boland

Bug Description

OK, so this was what I discovered in my attempt to land mir0.1.6 in archive
along with rebuilt components.
The combination is the latest touch image (devel-proposed)
rebuilt trunks of mir, platform-api, unity-system-compositor, unity-mir
using this list of MP's
https://code.launchpad.net/~mir-team/mir/trunk-0.1.6/+merge/208717
https://code.launchpad.net/~kgunn72/platform-api/papi-mir-0.1.6/+merge/208820
https://code.launchpad.net/~mterry/unity-system-compositor/greeter-depth/+merge/207549
https://code.launchpad.net/~kgunn72/unity-system-compositor/usc-mir-0.1.6/+merge/208821
https://code.launchpad.net/~robert-ancell/unity-system-compositor/usc-config-usr-share/+merge/200610
https://code.launchpad.net/~albaguirre/unity-mir/cross-compile-link-fix/+merge/205690
https://code.launchpad.net/~albaguirre/unity-mir/add-cross-compile-scripts/+merge/206058
https://code.launchpad.net/~andreas-pokorny/unity-mir/fix-1240400/+merge/207302
https://code.launchpad.net/~mterry/unity-mir/drop-dbusscreen/+merge/202236
https://code.launchpad.net/~mzanetti/unity-mir/expose-appimage-sourceSize/+merge/207626
https://code.launchpad.net/~gerboland/unity-mir/namespace-to-prevent-collision/+merge/208645
https://code.launchpad.net/~kgunn72/unity-mir/um-mir-0.1.6/+merge/208819

I also verified that this bug did not exist on the virgin image.

To reproduce the issue
- after flashing image, make r/w, connect to network & updating all the mir related components (warning: don't install libmirclientplatform-mesa or libmirplatformgraphics-mesa)
- go to dash, pick an app to install from the "suggested apps" list
- it should prompt you to "go to account info"
- select it, the setting app will start in accounts, pick ubuntu1
- select the text field for your user name, type it, now try to pick the password box - you will see you are stuck & you cannot x out your text either, as if OSK has stolen focus for good

I did do subsequent builds minus https://code.launchpad.net/~andreas-pokorny/unity-mir/fix-1240400/+merge/207302, but the issue remained. I tried a build minus all unity-mir mp's except for https://code.launchpad.net/~kgunn72/unity-mir/um-mir-0.1.6/+merge/208819 but it wouldn't boot.

anyway - until this is fixed, we are blocked from promoting mir0.1.6

Related branches

kevin gunn (kgunn72) on 2014-03-06
Changed in mir:
status: New → Triaged
importance: Undecided → Critical
kevin gunn (kgunn72) on 2014-03-06
Changed in unity-mir:
status: New → Triaged
importance: Undecided → Critical
Daniel van Vugt (vanvugt) wrote :

Mir 0.1.6 contains no changes to input/focus logic:
https://launchpad.net/mir/+milestone/0.1.6

However there is one such change coming in 0.1.7 that sounds related: bug 1268819
I think it's possible, even likely, that you you have the fix for bug 1268819 in Mir then you'll need to adjust all existing input logic (which probably contained workarounds until now).

I'm not ruling Mir out. We just need more investigation before saying for sure there's a problem in Mir.

Changed in mir:
status: Triaged → Incomplete
milestone: none → 0.1.7
Daniel van Vugt (vanvugt) wrote :

Hmm, the trunk-0.1.6 Mir merge does not contain the fix for bug 1268819. So I that's probably not relevant.

kevin gunn (kgunn72) wrote :
kevin gunn (kgunn72) wrote :

don't forget when installing the ppa...use sudo apt-get install libmirplatformgraphics-android libmirclientplatform-android libmirplatform

not dist-upgrade to avoid pulling in the libmirplatformgraphics-mesa & libmirclientplatform-mesa

kevin gunn (kgunn72) wrote :

http://www.youtube.com/watch?v=jsE2GiaSDp0

a little video in case you don't get a chance to repro

Daniel van Vugt (vanvugt) wrote :

I heard recently that the OSK is actually a fullscreen surface still. That's not only going to limit rendering performance but it means the OSK is responsible for passing through touch events above it to the surfaces below. I'll also see if Mir 0.1.6 contains any changes related to that.

Daniel van Vugt (vanvugt) wrote :

Also, is this bug really new? I've experienced random focus bugs and getting stuck for some time...

Alberto Aguirre (albaguirre) wrote :

Note that you need https://code.launchpad.net/~gerboland/unity-mir/namespace-to-prevent-collision/+merge/208645
otherwise there's symbol collisions (MirSurface) and unity8 won't start (bug #1282248 )

Also note that https://code.launchpad.net/~gerboland/unity-mir/namespace-to-prevent-collision/+merge/208645 is based on top of https://code.launchpad.net/~andreas-pokorny/unity-mir/fix-1240400/+merge/207302 so if you are merging namespace-to-prevent-collision it will bring fix-1240400.

Using https://code.launchpad.net/~kgunn72/unity-mir/um-mir-0.1.6/+merge/208819 with a minimal fix for bug #1282248 I don't see the focus issue.

After merging https://code.launchpad.net/~andreas-pokorny/unity-mir/fix-1240400/+merge/207302 I can reproduce the issue.

Daniel van Vugt (vanvugt) wrote :

Trying to reduce the number of variables I've started fresh with:
   vanilla devel-proposed + (custom unity-mir + fix-1240400)
No bug yet. So that branch is not responsible, at least not solely.

Michał Sawicz (saviq) wrote :

Info point: there's an OSKController component in unity-mir that's used to communicate the keyboard rectangle between keyboard and shell and set up input areas accordingly, since OSK itself can't do this IIUC.

Maybe something with input areas got b0rked, then?

Daniel van Vugt (vanvugt) wrote :

In the Mir update diff:
    https://code.launchpad.net/~mir-team/mir/trunk-0.1.6/+merge/208717
I can't see any changes related to focus or input, except that:
   InputSurface::position() is now called InputSurface::top_left()
However looking at the code in lp:unity-mir, that does not appear to be relevant.

Sanity check: Has _anything_ related to OSK logic changed in the past few weeks? I'm not familiar with where it all lives.

Daniel van Vugt (vanvugt) wrote :

Please double check the relevant projects that you are ready for this change in Mir 0.1.6 (which I wasn't aware of till now so it wasn't release noted):

--- include/server/mir/input/surface.h 2014-01-13 06:12:33 +0000
+++ include/server/mir/input/surface.h 2014-03-04 04:18:55 +0000
@@ -31,7 +31,7 @@
 {
 public:
     virtual std::string const& name() const = 0;
- virtual geometry::Point position() const = 0;
+ virtual geometry::Point top_left() const = 0;
     virtual geometry::Size size() const = 0;
     virtual bool contains(geometry::Point const& point) const = 0;

If you implement "position()" still then your code won't get called. That could be a problem.

kevin gunn (kgunn72) wrote :

So i saved the debs from the silo build, which should enable us to rebuild/debug unity-mir as we suspect its some combination of those MPs
https://drive.google.com/a/canonical.com/?tab=wo#folders/0B4GvOYxwuvpFdm5DbXNYUzFDdG8

Alberto Aguirre (albaguirre) wrote :

Well it turns out it's not fix-1240400, but instead the namespace change in, https://code.launchpad.net/~gerboland/unity-mir/namespace-to-prevent-collision/+merge/208645

apparently Q_PROPERTY definitions need the full namespace, they don't assume the namespace they are embedded in:
https://bugreports.qt-project.org/browse/QTBUG-15459

So Q_PROPERTY(MirSurface* ....) in the InputArea class needs to be
Q_PROPERTY(unitymir::MirSurface* ....)

So I suppose without it, qt reported no errors as it probably resolved to mir's MirSurface.

Changed in mir:
status: Incomplete → Invalid
Changed in unity-mir:
status: Triaged → In Progress
Changed in unity-mir:
assignee: nobody → Gerry Boland (gerboland)
kevin gunn (kgunn72) on 2014-03-07
Changed in unity-mir:
status: In Progress → Fix Committed
Changed in mir:
milestone: 0.1.7 → none
Daniel van Vugt (vanvugt) wrote :

In Progress. The branch apparently has not landed yet.

Changed in unity-mir:
status: Fix Committed → In Progress
kevin gunn (kgunn72) on 2014-03-11
Changed in unity-mir:
status: In Progress → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers