Created attachment 277045 [details] [review]
Patch for keyboard.js

Attached the patch.

Created attachment 289712 [details] [review]
Patch for keyboard.js

Updated the patch for HEAD.
Currently I have no idea to bring back the ibus engine by manual.

Review of attachment 289712 [details] [review]:

Please include the file you're modifying in the commit subject, e.g. ibusManager: ... or status/keyboard: ...

I think a more elegant way to do this is to add a flag like InputSourceManager._disableIBus or so and then set or unset it when we get the set-content-type signal according to the input purpose and just call InputSourceManager.reload().

Then, when building the runtime list of input sources in _inputSourcesChanged() we can skip the IBus sources when that flag says so.

Created attachment 291048 [details] [review]
Patch for ibusManager.js

Revised the patch.
This patch can disable IMEs on password dialogs.
Super+space shows XKB only but right click on panel shows all engines.

Review of attachment 291048 [details] [review]:

The subject would be more accurate written as "... Disable IBus input sources on password entries"

Note that for this to work correctly we'll need an ibus fix. See

https://codereview.appspot.com/176260043/

::: js/misc/ibusManager.js
@@ +126,3 @@
             this._candidatePopup.setPanelService(this._panelService);
             this._panelService.connect('update-property', Lang.bind(this, this._updateProperty));
+            this._panelService.connect('set-content-type', Lang.bind(this, this._setContentType));

We should only connect this signal if the ibus version is new enough to include the fix I mentioned above.

::: js/ui/status/keyboard.js
@@ +157,2 @@
         this._currentSource = null;
+        this._backupSourceForPassword = null;

Let's just call this _backupSource

@@ +401,3 @@
+                this._mruSources[0].activate();
+            }
+        }

This can more succinctly be written like:

        if (!this._disableIBus && this._backupSource) {
            for (let i = 0; i < this._mruSources.length; i++) {
                if (this._mruSources[i].type == this._backupSource.type &&
                    this._mruSources[i].id == this._backupSource.id) {                                                              
                    let currentSource = this._mruSources.splice(i, 1);
	            this._mruSources = currentSource.concat(this._mruSources);
                    break;
                }
            }
            this._backupSource = null;
	}

and leave the activate() call as it is.

@@ +460,3 @@
+    _ibusSetContentType: function(im, purpose, hints) {
+        if (purpose == IBus.InputPurpose.PASSWORD ||
+            purpose == IBus.InputPurpose.PIN ) {

Why PIN ?

@@ +470,3 @@
+            }
+            if (!hasXkbSources)
+                return;

All of this hasXkbSources check can be as simple as:

	    if (Object.keys(this._inputSources).length == Object.keys(this._ibusSources).length)
                return;

@@ +481,3 @@
+            this._disableIBus = false;
+        }
+        this._mruSources = [];

Clearing _mruSources doesn't seem to needed. Did you think of any special reason to do it?

Created attachment 291617 [details] [review]
Patch for ibusManager.js

Updated the patch.

(In reply to comment #5)
> Review of attachment 291048 [details] [review]:
> @@ +460,3 @@
> +    _ibusSetContentType: function(im, purpose, hints) {
> +        if (purpose == IBus.InputPurpose.PASSWORD ||
> +            purpose == IBus.InputPurpose.PIN ) {
> 
> Why PIN ?

I thought PIN is a secure key. OK, I removed it.

> @@ +470,3 @@
> +            }
> +            if (!hasXkbSources)
> +                return;
> 
> All of this hasXkbSources check can be as simple as:
> 
>         if (Object.keys(this._inputSources).length ==
> Object.keys(this._ibusSources).length)
>                 return;

Great. Thanks.

> @@ +481,3 @@
> +            this._disableIBus = false;
> +        }
> +        this._mruSources = [];
> 
> Clearing _mruSources doesn't seem to needed. Did you think of any special
> reason to do it?

I added some comments in the source file.
If this._mruSources is not cleared, IM sources will be appended to XKB sources after the lock screen is unlocked.

E.g. the order is IM1, XKB1, IM2, XKB2, IM3 before lock the screen and it will be IM1, XKB1, XKB2, IM2, IM3 after unlock the screen.

Review of attachment 291617 [details] [review]:

With the comment below addressed this is a-c-n. Thanks

::: js/misc/ibusManager.js
@@ +126,3 @@
+                this._panelService.connect('set-content-type', Lang.bind(this, this._setContentType));
+            } catch (e) {
+                log('Disable set-content-type signal: ' + e);

There's no need to log this IMO, most of the functionality is still working anyway so let's remain silent here.

OTOH, I'd like to see a comment here and also in the commit message explaining why we don't do this for older ibus versions.