Per-window input sources option does not work on Bionic with gnome-flashback

Bug #1769838 reported by DEXTER
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
gnome-flashback (Ubuntu)
Fix Released
Undecided
Unassigned
Bionic
Fix Released
Undecided
Unassigned
metacity (Ubuntu)
Fix Released
Undecided
Unassigned
Bionic
Fix Released
Undecided
Unassigned

Bug Description

[Impact]

 * An explanation of the effects of the bug on users and:
   Setting the 'Input Source Options' to 'Allow different sources for each window' does not work.

 * justification for backporting the fix to the stable release:
   This is a bug in Ubuntu Bionic's version of gnome-flashback (3.28) (because the GUI lets the user change the input source option but the backend does nothing)

[Test Case]

 * detailed instructions how to reproduce the bug
   - Set Input Source Options to Allow different sources for each window in gnome-control-center -> Region & Language -> Options
   - Realize that if you set different layouts for different windows it does not change automatically when changing windows.

[Regression Potential]

* Metacity now ignores its own events when predicting focus changes. Its own events are recognized by comparing timestamps and serial numbers. If something potentially goes wrong, it can either not ignore its own events, or ignore external events. In both cases the focus prediction will be broken. Also there could be potentially a race condition, but the patch protects against it by making a dummy request with bumped serial number. (Note: the second metacity patch is an amendment for the first one, so the previous analysis applies to both patches.)

* GNOME-Flashback has some new code for handling per-window input sources. The change_per_window_source() function returns early if the sources_per_window option (obtained from GSettings: org.gnome.desktop.input-sources per-window) is false. As that option is false by default, in the default configuration most of the new code won't be executed at all. The potential breakage may happen if it is set to true. Such potential breakage includes: wrong input sources handling (it was wrong before anyway), gnome-flashback crashes. The patches are already applied in Cosmic, and currently no crashes are reported against the Cosmic version of gnome-flashback on errors.ubuntu.com.

[Other Info]

This needs fixes in both gnome-flashback and metacity.
Here are the relevant commits in gnome-3-28 branches:

https://gitlab.gnome.org/GNOME/metacity/commit/b96341dabffc3589 (ensure that we ignore our own focus events for focus predictions)
https://gitlab.gnome.org/GNOME/metacity/commit/9956d376d38d0ad6 (fix problems with focus tracking)
https://gitlab.gnome.org/GNOME/gnome-flashback/commit/3c4c6ecddef48cd5 (implement per window input sources)

The gnome-session-flashback dependency on metacity will be bumped.

Rhonda D'Vine (rhonda)
Changed in pkg-website:
status: New → Invalid
no longer affects: pkg-website
Revision history for this message
Alberts Muktupāvels (muktupavels) wrote :

Oh, this is upstream bug! Different sources for each window is not implemented... :(

Revision history for this message
DEXTER (mydexterid) wrote :

Can you please link the upstream bug, or the source where I can look into it further?

This is very annoying for me, and if it is not very hard I could try fixing it.

Revision history for this message
Alberts Muktupāvels (muktupavels) wrote :

I think there is no upstream bug, but you can fill one:
https://bugzilla.gnome.org/enter_bug.cgi?product=gnome-flashback

But then maybe don't waste time as all modules will be moved to gitlab and I don't plan to ask for bug migration for gnome-flashback modules. So might be better to wait until modules are moved and then open bug in gitlab...

Migration plan:
https://mail.gnome.org/archives/desktop-devel-list/2018-March/msg00023.html

https://git.gnome.org/browse/gnome-flashback/tree/gnome-flashback/libinput-sources
This is place where you should look. There is gnome-flashback channel on irc.gnome.org, if you want ask something.

There are two FIXMEs:
- https://git.gnome.org/browse/gnome-flashback/tree/gnome-flashback/libinput-sources/gf-input-source-manager.c#n103
- https://git.gnome.org/browse/gnome-flashback/tree/gnome-flashback/libinput-sources/gf-input-source-manager.c#n955

Input sources was ported / adapted from gnome-shell:
https://gitlab.gnome.org/GNOME/gnome-shell/blob/master/js/ui/status/keyboard.js
(note that there might have been many changes, you might need to look at gnome-3-16 or 3-18 (gnome-shell) branch, because input-sources first was added in gnome-flashback 3.18)

_sourcesPerWindowChanged in js should be per_window_changed_cb and _changePerWindowSource should be change_per_window_source.

Revision history for this message
DEXTER (mydexterid) wrote :

Thanks!

Looking at these briefly I can see why this was not implemented at first.
As far as I can tell it is hard to tell which window has the focus right now.

The .js version uses libmutter mutter-3.28.1/src/core/display-private.h, which has an interesting comment on this:
" /* Our best guess as to the "currently" focused window (that is, the
   * window that we expect will be focused at the point when the X
   * server processes our next request), and the serial of the request
   * or event that caused this.
   */
  MetaWindow *focus_window;
"
so it is already a best guess only. Maybe gnome-flashback should depend on libmutter and use this MetaDisplay's MetaWindow to get the currently focused window? Or maybe there is an easier way using gdk/gtk to get the currently focused window?

IDK. Unfortunately this does not seem to be a trivial issue unfortunately...

Revision history for this message
DEXTER (mydexterid) wrote :

(I don't know how to edit a comment so here's a new one)

Should have read more about this mutter thingy. It seems to be a window manager for Gnome 3, which is metacity for Gnome 2. So libmutter could not be used for gnome-flashback probably...

Revision history for this message
Alberts Muktupāvels (muktupavels) wrote :

I think you can not edit comments. Yes, mutter is window manager and we are duplicating some functionality that is in mutter and/or in gnome-shell.

I don't remember why it was not implemented, but we should be able to use gdk and/or x11 to know focused window.

https://standards.freedesktop.org/wm-spec/wm-spec-1.4.html#idm139915844368784
_NET_ACTIVE_WINDOW probably should be enough. If per window input sources are enabled then we should listen to this property changes to know when active window changes and do same that is done in _setPerWindowInputSource.

So we probably should subscribe to related events on root window. Using gdk_window_add_filter add callback. If it is property change event (might be wrong) then use gdk_screen_get_active_window to get active window:
https://developer.gnome.org/gdk3/stable/GdkScreen.html#gdk-screen-get-active-window

If you really want to try I would suggest you to use IRC to ask more specific questions and I will try to help. If you dont use IRC then maybe skype?

Revision history for this message
Alberts Muktupāvels (muktupavels) wrote :
Revision history for this message
DEXTER (mydexterid) wrote :

Sorry, I did not have the free time to bother with this. I'm using a console hack/script to do the switching for now.

I reported it here: https://gitlab.gnome.org/GNOME/gnome-flashback/issues/5

Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package gnome-flashback - 3.30.0-1ubuntu1

---------------
gnome-flashback (3.30.0-1ubuntu1) cosmic; urgency=medium

  * Merge with Debian unstable, remaining changes:
    - debian/rules:
      + Add an epoch number to gnome-session-flashback package.
    - debian/gnome-flashback-common.gsettings-override:
      + Disable some not needed gnome-flashback components.
      + Add the default settings for GNOME Flashback sessions.
    - debian/gnome-session-flashback.target:
      + The target file for systemd, based on ubuntu-session.target.
      + Install this file into /usr/lib/systemd/user/ directory.
    - debian/control.in:
      + Bump gnome-session-bin dependency to 3.18.1.2-1ubuntu5, to be able
        to use run-systemd-session.
      + Add dependencies on dbus-user-session, indicator-common, nautilus
        and systemd.
      + Use Ubuntu VCS fields.
    - debian/patches/run-systemd-session.diff:
      + Use run-systemd-session script for running the session.
    - debian/patches/support-indicator-keyboard.diff:
      + Support “org.gnome.desktop.input-sources current” key, to make
        indicator-keyboard working.
    - debian/patches/revert-nautilus-remove.diff:
      + Revert removing nautilus-classic from required components. Ubuntu
        18.10 ships Nautilus 3.26, which still has desktop icons support.
  * The new release brings the following features:
    - Accessibility keyboard plugin, supporting sticky keys (LP: #1790744).
    - Per-window input sources support (LP: #1769838).
  * Drop assign-groups-in-order.diff, applied in the new release.
  * Drop trackpoint-settings.diff, applied in the new release.
  * Refresh debian/patches/support-indicator-keyboard.diff.
  * Add epoch to the required metacity version (1:3.30.1).
  * Remove button-layout override which is now present upstream.

 -- Dmitry Shachnev <email address hidden> Mon, 10 Sep 2018 10:47:44 +0300

Changed in gnome-flashback (Ubuntu):
status: New → Fix Released
Revision history for this message
DEXTER (mydexterid) wrote :

How can I set this issue to show that it affects Ubuntu Bionic (that's what I reported in the first place) and it is still not fixed for Bionic (only on Cosmic)

Revision history for this message
Dmitry Shachnev (mitya57) wrote :

Hi Dexter!

If you want to get this fixed in Bionic, please update the bug description to match this template: <https://wiki.ubuntu.com/StableReleaseUpdates#SRU_Bug_Template>.

Then I will include it in my next gnome-flashback Bionic upload after the current one (3.28.0-1ubuntu1.1) migrates from -proposed to -updates.

DEXTER (mydexterid)
description: updated
Changed in metacity (Ubuntu):
status: New → Fix Released
Revision history for this message
Dmitry Shachnev (mitya57) wrote :

This needs also a fix in metacity, updated the bug information accordingly.

description: updated
Revision history for this message
Łukasz Zemczak (sil2100) wrote :

Thank you for submitting the SRU! I think I would need a bit more regression potential analysis for code changes like these. The regression potential field's purpose is to analyse which parts of the system can be potentially broken by the SRUed changes, for instance - seeing the metacity changes modifying parts of focus handling, a guess for regression potential could be that focus handling in overall could be broken, etc.

Could you modify the regression potential field with results of such investigation? Thank you.

Revision history for this message
Dmitry Shachnev (mitya57) wrote :

Hi Łukasz, now it should be better.

description: updated
summary: - Input source options does not work on Bionic with gnome-flashback
+ Per-window input sources option does not work on Bionic with gnome-
+ flashback
Revision history for this message
Łukasz Zemczak (sil2100) wrote : Please test proposed package

Hello DEXTER, or anyone else affected,

Accepted metacity into bionic-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/metacity/1:3.28.0-1ubuntu0.1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested and change the tag from verification-needed-bionic to verification-done-bionic. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-bionic. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

Changed in metacity (Ubuntu Bionic):
status: New → Fix Committed
tags: added: verification-needed verification-needed-bionic
Revision history for this message
Łukasz Zemczak (sil2100) wrote :

Hello DEXTER, or anyone else affected,

Accepted gnome-flashback into bionic-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/gnome-flashback/3.28.0-1ubuntu1.2 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested and change the tag from verification-needed-bionic to verification-done-bionic. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-bionic. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

Changed in gnome-flashback (Ubuntu Bionic):
status: New → Fix Committed
Revision history for this message
DEXTER (mydexterid) wrote :

Updated the packages to the proposed versions. The fixes work for me.

Thank you, great job!

ii gnome-flashback 3.28.0-1ubuntu1.2 amd64
ii metacity 1:3.28.0-1ubuntu0.1 amd64

tags: added: verification-done-bionic
removed: verification-needed-bionic
tags: added: verification-done
removed: verification-needed
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package metacity - 1:3.28.0-1ubuntu0.1

---------------
metacity (1:3.28.0-1ubuntu0.1) bionic; urgency=medium

  * Backport two commits from upstream gnome-3-28 branch to fix issues
    with focus handling, needed for per-window input source support in
    GNOME Flashback (LP: #1769838):
    - focus_events.diff: ensure that we ignore our own focus events for
      focus predictions;
    - focus_tracking.diff: fix problems with focus tracking.

 -- Dmitry Shachnev <email address hidden> Thu, 13 Sep 2018 13:00:14 +0300

Changed in metacity (Ubuntu Bionic):
status: Fix Committed → Fix Released
Revision history for this message
Łukasz Zemczak (sil2100) wrote : Update Released

The verification of the Stable Release Update for metacity has completed successfully and the package has now been released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.

Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package gnome-flashback - 3.28.0-1ubuntu1.2

---------------
gnome-flashback (3.28.0-1ubuntu1.2) bionic; urgency=medium

  * Backport upstream patch to apply mouse speed and natural scroll
    settings also to trackpoints (trackpoint-settings.diff; LP: #1790336).
  * Backport upstream patch to implement per-window input sources
    (per-window-input-sources.diff; LP: #1769838).
  * Bump required metacity version to 1:3.28.0-1ubuntu0.1.

 -- Dmitry Shachnev <email address hidden> Thu, 13 Sep 2018 13:52:29 +0300

Changed in gnome-flashback (Ubuntu Bionic):
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.