From 65f96732fba480f1d6e0ce760d8666cbbf5abfd3 Mon Sep 17 00:00:00 2001 From: Ray Strode Date: Mon, 22 Jul 2019 10:57:57 -0400 Subject: [PATCH 05/13] appDisplay: Add destroy handler for FolderIcon It is important that the FolderView of a FolderIcon always gets destroyed before the AppFolderPopup, since the view may or may not be in the popup, and the view should get cleaned up exactly once in either case. This commit adds a destroy handler on FolderIcon to ensure things get taken down in the right order, and to make sure the view isn't leaked if it's not yet part of the popup. https://gitlab.gnome.org/GNOME/gnome-shell/merge_requests/628 --- js/ui/appDisplay.js | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/js/ui/appDisplay.js b/js/ui/appDisplay.js index 756e8ae44..2392281a2 100644 --- a/js/ui/appDisplay.js +++ b/js/ui/appDisplay.js @@ -1275,69 +1275,77 @@ var FolderIcon = new Lang.Class({ Name: 'FolderIcon', _init(id, path, parentView) { this.id = id; this.name = ''; this._parentView = parentView; this._folder = new Gio.Settings({ schema_id: 'org.gnome.desktop.app-folders.folder', path: path }); this.actor = new St.Button({ style_class: 'app-well-app app-folder', button_mask: St.ButtonMask.ONE, toggle_mode: true, can_focus: true, x_fill: true, y_fill: true }); this.actor._delegate = this; // whether we need to update arrow side, position etc. this._popupInvalidated = false; this.icon = new IconGrid.BaseIcon('', { createIcon: this._createIcon.bind(this), setSizeManually: true }); this.actor.set_child(this.icon.actor); this.actor.label_actor = this.icon.label; this.view = new FolderView(); this.actor.connect('clicked', () => { this._ensurePopup(); this.view.actor.vscroll.adjustment.value = 0; this._openSpaceForPopup(); }); + this.actor.connect('destroy', this.onDestroy.bind(this)); this.actor.connect('notify::mapped', () => { if (!this.actor.mapped && this._popup) this._popup.popdown(); }); this._folder.connect('changed', this._redisplay.bind(this)); this._redisplay(); }, + onDestroy() { + this.view.actor.destroy(); + + if (this._popup) + this._popup.actor.destroy(); + }, + getAppIds() { return this.view.getAllItems().map(item => item.id); }, _updateName() { let name = _getFolderName(this._folder); if (this.name == name) return; this.name = name; this.icon.label.text = this.name; this.emit('name-changed'); }, _redisplay() { this._updateName(); this.view.removeAll(); let excludedApps = this._folder.get_strv('excluded-apps'); let appSys = Shell.AppSystem.get_default(); let addAppId = appId => { if (this.view.hasItem(appId)) return; if (excludedApps.indexOf(appId) >= 0) return; let app = appSys.lookup_app(appId); if (!app) @@ -1460,61 +1468,60 @@ var AppFolderPopup = new Lang.Class({ // We don't want to expand really, but look // at the layout manager of our parent... // // DOUBLE HACK: if you set one, you automatically // get the effect for the other direction too, so // we need to set the y_align x_expand: true, y_expand: true, x_align: Clutter.ActorAlign.CENTER, y_align: Clutter.ActorAlign.START }); this._boxPointer = new BoxPointer.BoxPointer(this._arrowSide, { style_class: 'app-folder-popup-bin', x_fill: true, y_fill: true, x_expand: true, x_align: St.Align.START }); this._boxPointer.actor.style_class = 'app-folder-popup'; this.actor.add_actor(this._boxPointer.actor); this._boxPointer.bin.set_child(this._view.actor); this.closeButton = Util.makeCloseButton(this._boxPointer); this.closeButton.connect('clicked', this.popdown.bind(this)); this.actor.add_actor(this.closeButton); this._boxPointer.actor.bind_property('opacity', this.closeButton, 'opacity', GObject.BindingFlags.SYNC_CREATE); global.focus_manager.add_group(this.actor); - source.actor.connect('destroy', () => { this.actor.destroy(); }); this._grabHelper = new GrabHelper.GrabHelper(this.actor); this._grabHelper.addActor(Main.layoutManager.overviewGroup); this.actor.connect('key-press-event', this._onKeyPress.bind(this)); this.actor.connect('destroy', this._onDestroy.bind(this)); }, _onDestroy() { if (this._isOpen) { this._isOpen = false; this._grabHelper.ungrab({ actor: this.actor }); this._grabHelper = null; } }, _onKeyPress(actor, event) { if (global.stage.get_key_focus() != actor) return Clutter.EVENT_PROPAGATE; // Since we need to only grab focus on one item child when the user // actually press a key we don't use navigate_focus when opening // the popup. // Instead of that, grab the focus on the AppFolderPopup actor // and actually moves the focus to a child only when the user // actually press a key. // It should work with just grab_key_focus on the AppFolderPopup // actor, but since the arrow keys are not wrapping_around the focus // is not grabbed by a child when the widget that has the current focus // is the same that is requesting focus, so to make it works with arrow // keys we need to connect to the key-press-event and navigate_focus // when that happens using TAB_FORWARD or TAB_BACKWARD instead of arrow -- 2.25.1