appDisplay: Handle an error moving nonexistent app icons #754
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Some gstreamer plugins require a connection to the display server, so they block until the server is up and running. That's why we moved the check into the D-Bus service, so that the blocking would not lock up the compositor itself. However the block can still delay the service initialization so much that auto-shutdown hits immediately when returning from the constructor. If that happens, the proxy on the shell side is no longer backed by a remote object when the init callback runs, and all properties therefore resolve as `null`. As a result, gnome-shell thinks that screencasts aren't supported and hides the screencast button. Fix this by holding the service during the gstreamer checks, so that the auto-shutdown timeout only starts after the service is ready. https://gitlab.gnome.org/GNOME/gnome-shell/-/issues/6051 Part-of: <https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/2533>
The screencast service shuts down when not in use, so it is almost certainly not running when handling the screencast shortcut. Instead of making sure the service is restarted, just cache the property when initializing the proxy. Part-of: <https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/2533>
Each permission entry is an array of strings, so checking that against the expected string itself will always fail. https://gitlab.gnome.org/GNOME/gnome-shell/-/issues/6107 Part-of: <https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/2548> (cherry picked from commit b0befbb)
…g focus The default StButton event handlers will create a temporary grab that will steal the keyboard focus out of the client, and cause it to pop down the candidates window. Avoid the default grabbing behavior, so that focus stays on the Wayland client while interacting with the ibusCandidatesPopup with the pointer. Closes: https://gitlab.gnome.org/GNOME/mutter/-/issues/2244 Part-of: <https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/2497>
The quick settings menu is child of an actor at 0,0 coordinates, simply transformed to look in place. However the parent actor stays at 0,0, and gets a width/height that is able to contain the child menu (even though the transform will push it outside the parent actor). This makes the parent actor (not meant to handle pointer/touch input) to cover parts of the shell. Since this actor is also where the Clutter grab happens, the menu is not dismissed either. Make this parent actor have a 0x0 size, so that it can still act as the parent for the quick settings menu, but does not actually cover any area. This makes clicks around the top left corner work again while the quick settings menu is opened. Closes: https://gitlab.gnome.org/GNOME/gnome-shell/-/issues/5887 Part-of: <https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/2535> (cherry picked from commit bcafa51)
When the overview is hidden just after starting GNOME Shell the app grid is not populated yet. Thus, this._grid.nPages is 0. When the overview is about to be hidden, we call goToPage(0) on the AppGrid. This results in pageNumber being clamped to -1 and leads to the following exception: ``` JS ERROR: Exception in callback for signal: hidden: Error: Page -2 does not exist at IconGrid getItemsAtPage@resource:///org/gnome/shell/ui/iconGrid.js:1383:19 _translatePreviousPageIcons@resource:///org/gnome/shell/ui/appDisplay.js:323:34 _syncPageIndicators@resource:///org/gnome/shell/ui/appDisplay.js:396:14 goToPage@resource:///org/gnome/shell/ui/appDisplay.js:445:14 goToPage@resource:///org/gnome/shell/ui/appDisplay.js:1237:29 goToPage@resource:///org/gnome/shell/ui/appDisplay.js:1584:15 _init/<@resource:///org/gnome/shell/ui/appDisplay.js:1328:52 _emit@resource:///org/gnome/gjs/modules/core/_signals.js:89:42 _hideDone@resource:///org/gnome/shell/ui/overview.js:589:18 _animateNotVisible/<@resource:///org/gnome/shell/ui/overview.js:566:55 onStopped@resource:///org/gnome/shell/ui/overviewControls.js:753:21 _makeEaseCallback/<@resource:///org/gnome/shell/ui/environment.js:150:22 _easeActorProperty/<@resource:///org/gnome/shell/ui/environment.js:316:60 ``` Connect to the overview hidden signal only after initializing the AppDisplay content, so that goToPage(0) is not called when there is no page yet. Closes: https://gitlab.gnome.org/GNOME/gnome-shell/-/issues/5837 Part-of: <https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/2536> (cherry picked from commit 2326721)
Even though the emoji panel may be destroyed with the OSK, the swipeTracker that is set up to navigate between pages is left lingering, and handling events for some gestures in the stage. This results in warnings like: JS ERROR: TypeError: this._panel is null set delta@resource:///org/gnome/shell/ui/keyboard.js:720:9 _onSwipeUpdate@resource:///org/gnome/shell/ui/keyboard.js:750:22 _updateGesture@resource:///org/gnome/shell/ui/swipeTracker.js:670:14 vfunc_gesture_progress@resource:///org/gnome/shell/ui/swipeTracker.js:288> on touch interaction after the OSK was shown and dismissed. In order to fix this, issue explicit destruction of the swipeTracker when the emoji pager actor is destroyed. Part-of: <https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/2543> (cherry picked from commit 7e4f7c7)
The swipeTracker may connect gestures on the stage, so we might end up in the odd situation that the swipeTracker is handling events for an invisible actor. Avoid doing this. Part-of: <https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/2543> (cherry picked from commit a0352ba)
…uction The OSK may be created and destroyed, while it connects to signals from the longer-lived InputMethod object. This makes these signals linger and issue copious warnings while trying to change the extended keymap and new ratio on a defunct OSK. Closes: https://gitlab.gnome.org/GNOME/gnome-shell/-/issues/6085 Part-of: <https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/2543> (cherry picked from commit 5225362)
If the OSK is destroyed while visible without being close()'d first, the completion mode might remain turned on. Ensure it is turned off on OSK destruction so that typing-booster has no chance to remain turned on. Part-of: <https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/2543> (cherry picked from commit a33e3ea)
We assign (disambiguated) device names every time a device is added or removed, and store the name on the corresponding menu item. However menu items are only created when the device should be shown, not necessarily when it is added (unplugged ethernet cable, ongoing initialization, ...). Fix this by tracking device names separately from device items, and set the name on newly created items. https://gitlab.gnome.org/GNOME/gnome-shell/-/issues/6040 Part-of: <https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/2529> (cherry picked from commit b2ff25b)
Commit 637ee73 accidentally dropped all parameters from the method invocation when changing a .bind() handler to an arrow function. Part-of: <https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/2545> (cherry picked from commit 14f061f)
We currently initialize regions in all code paths except for the greeter. But while there are no windows on the login screen, the work area can still be used for positioning, for example for notifications. Part-of: <https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/2546> (cherry picked from commit 990cdbd)
The indicator needs special styling for the color, but we still want it to pick shared 'system-status-icon' styling like the icon size. https://gitlab.gnome.org/GNOME/gnome-shell/-/issues/6008 Part-of: <https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/2547> (cherry picked from commit d0740d6)
We don't allow entering the overview when there's an ongoing grab operation, as they are generally incompatible with ClutterGrab. The exception are wayland popups which are expected to work, and don't conflict with ClutterGrabs. https://gitlab.gnome.org/GNOME/gnome-shell/-/issues/6092 Part-of: <https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/2549> (cherry picked from commit a4e2d70)
Move preparation code into their own functions. These functions will be used in order to separate the animation control into two steps. Part-of: <https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/2532> (cherry picked from commit b8b8919)
This give us more control on when the preparation happens and will be used in the subsequent commit. Part-of: <https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/2532> (cherry picked from commit f23ca85)
Since commit 7bd98f3 the onComplete callback is called right away when animations are disabled. As side effect, now this._coverPane gets shown/hidden in the wrong order. Start the animation after setting this._coverPane initial state, so that the animation callback can set the this._coverPane final state without it being overwritten later. Move the animation also after emitting the showing and hiding signals. So that the order showing -> shown and hiding -> hidden is still preserved when animations are disabled. Closes: https://gitlab.gnome.org/GNOME/gnome-shell/-/issues/6096 Part-of: <https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/2532> (cherry picked from commit cf47b4a)
Part-of: <https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/1349> (cherry picked from commit 6fc538b)
It was added, as there was a previous signal disconnect in this function, but it happened to be dead code, and the disconnectObject() call was added in error. Remove it. Fixes: 26235bb Part-of: <https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/1349> (cherry picked from commit 7f4db05)
Running dispose on the setting causes issues when we're being destroyed, as the signal handler in the GSettings binding will be cleared by the GObject itself before it has the chance, resulting in warnings. Part-of: <https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/1349> (cherry picked from commit fa47a75)
The nPages property can be 0, which is the case in a stripped down CI image without a single app to be listed, which means we can't clamp to `nPages - 1`, since that'll give us a negative index and the following error: (gnome-shell:266): Gjs-CRITICAL **: 16:48:56.633: JS ERROR: Error: Page -2 does not exist at IconGrid getItemsAtPage@resource:///org/gnome/shell/ui/iconGrid.js:1383:19 _translatePreviousPageIcons@resource:///org/gnome/shell/ui/appDisplay.js:323:34 _syncPageIndicators@resource:///org/gnome/shell/ui/appDisplay.js:396:14 goToPage@resource:///org/gnome/shell/ui/appDisplay.js:445:14 goToPage@resource:///org/gnome/shell/ui/appDisplay.js:1236:29 goToPage@resource:///org/gnome/shell/ui/appDisplay.js:1582:15 _init/<@resource:///org/gnome/shell/ui/appDisplay.js:499:18 _init/pagesChangedId<@resource:///org/gnome/shell/ui/iconGrid.js:1137:24 _removePage@resource:///org/gnome/shell/ui/iconGrid.js:454:14 _removeItemData@resource:///org/gnome/shell/ui/iconGrid.js:495:18 moveItem@resource:///org/gnome/shell/ui/iconGrid.js:854:14 moveItem@resource:///org/gnome/shell/ui/iconGrid.js:1277:29 _moveItem@resource:///org/gnome/shell/ui/appDisplay.js:1194:20 _redisplay/<@resource:///org/gnome/shell/ui/appDisplay.js:1085:22 _redisplay@resource:///org/gnome/shell/ui/appDisplay.js:1080:17 _redisplay@resource:///org/gnome/shell/ui/appDisplay.js:1370:15 _init/<@resource:///org/gnome/shell/ui/appDisplay.js:633:24 _initializeManager@resource:///org/gnome/shell/misc/parentalControlsManager.js:95:14 async*_init@resource:///org/gnome/shell/misc/parentalControlsManager.js:63:14 ParentalControlsManager@resource:///org/gnome/shell/misc/parentalControlsManager.js:55:4 getDefault@resource:///org/gnome/shell/misc/parentalControlsManager.js:42:22 start@resource:///org/gnome/shell/ui/main.js:184:29 @resource:///org/gnome/shell/ui/init.js:6:17 Part-of: <https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/1349> (cherry picked from commit 367d40d)
This avoids having animating timelines on invisible and detached actors. This also silences some warnings about the timeline being running without being attached to the stage. Part-of: <https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/1349> (cherry picked from commit 210754f)
Otherwise we end up accumulating devices when pipewire crashes or is restarted. https://gitlab.gnome.org/GNOME/gnome-shell/-/issues/6142 Part-of: <https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/2558> (cherry picked from commit 1b62b7e)
… shown If we are getting purpose hint changes while the language switcher is popped up, this likely means the purpose hint was actually triggered by the key focus change induced by the language switcher popping up. In this case, we on one hand would like to preserve the state that applied before thise focus change, and on the other we very much want to avoid the keymap change that would forget about the keys being pressed. Closes: https://gitlab.gnome.org/GNOME/gnome-shell/-/issues/6066 Part-of: <https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/2534>
We will differentiate some behavior depending on current desktop. Add an helper to centralize the current desktop detection. Forwarded: not-needed Origin: ubuntu =================================================================== (cherry picked from commit 2167aee)
Ensure that no update is proposed or loaded if sideloaded (always prefer system version) on the ubuntu session. We want to ensure that the default code running is going through our QA and security team process than being loaded from a 3rd party website. Also, that will enable us to upload newer versions on GNOME extension website while still letting older Endless release versions running expected extension version. Origin: ubuntu Modified by: Endless Forwarded: https://bugzilla.gnome.org/show_bug.cgi?id=789852 (cherry picked from commit 06b5af7)
Historically we’ve defined it as ‘endless’ (e.g. `XDG_CURRENT_DESKTOP=endless:GNOME`). We’re now looking to standardise the desktop ID, and for that it would be better if it were capitalised like other desktop IDs. See: https://gitlab.freedesktop.org/xdg/xdg-specs/-/merge_requests/73 Signed-off-by: Philip Withnall <[email protected]> https://phabricator.endlessm.com/T34910
This commit reimplements our password reset feature from eos-shell. It is much simpler than our original code, because gnome-shell's upstream login screen code is far simpler and less fragile than it used to be. From 3.3, we removed phone numbers for password resets since at this point, unless the user has a local support number via other means, we only globally advertise email addresses for customer support. As for the keyfile, look for it in multiple locations defined by the search path and load and cache (and return) the first keyfile found. The function assumes the keyfile will be found somewhere, in that it will attempt to find one every time it is called. The function also doesn't merge keys from different keyfiles - only the first one found is used. All entries from other v-c-s.ini files in other locations (if any) are ignored. Last, this implementation adds password unlock code salting that is used if it is defined in the "Password Reset" section of the the vendor-customer-support.ini keyfile. The algorithm adds a version-specific prefix to the password reset code. This allows us to immediately notice when the wrong tool is used to generate unlock code and avoid confusion. * 2020-03-25: - Squashed with 7e99aee - Squashed with 4713f33 * 2020-08-18: - Squashed with 58913a7 * 2020-09-23: - Updated password reset instructions to use new web service * 2023-11-23: - Removed duplicate definition of PKGDATADIR https://phabricator.endlessm.com/T17245 https://phabricator.endlessm.com/T19035 https://phabricator.endlessm.com/T20297 https://phabricator.endlessm.com/T30648 https://phabricator.endlessm.com/T30685 https://phabricator.endlessm.com/T35039
This helps users to remember their passwords without needing to call customer support. https://phabricator.endlessm.com/T17245
The tool was added in 2018 to migrate to per-desktop overrides from the old overrides system. 5 years later, everyone who’s going to migrate probably has migrated, so we can delete the script and remove a process running on every login. Signed-off-by: Philip Withnall <[email protected]> Upstream: https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/2611 https://phabricator.endlessm.com/T34300
See the comment in the new `gnome-shell-overrides-reset.sh` script for details. This commit is specific to EOS. It can be dropped any time after the first checkpoint after the EOS 5 release. By that point, any users who have upgrade from 4 → 5 (and triggered the bug this script fixes) should have logged into EOS 5 at least once, hence run this script, and the migration bug should be fixed for them. Signed-off-by: Philip Withnall <[email protected]> https://phabricator.endlessm.com/T34300
This uses EOS metrics recorder daemon, so add the dependency too. https://phabricator.endlessm.com/T33252 https://phabricator.endlessm.com/T35039
An Endless OS user will likely not be familiar with the term GNOME.
I took a quick look at whether we could use some field of
/etc/os-release rather than hardcoding "Endless OS 5" here. The short
answer is no. All fields of os-release that have a version number
contain the full x.y.z.
PRETTY_NAME is supposed to be:
> A pretty operating system name in a format suitable for presentation
> to the user. May or may not contain a release code name or OS version
> of some kind, as suitable.
>
> Example: "PRETTY_NAME="Fedora 17 (Beefy Miracle)"".
Ours is:
PRETTY_NAME="Endless OS 5.0.0"
But if we change this to "Endless OS 5", the full 5.y.z version number
will no longer be visible in Settings → About. The BUILD_ID (OSTree
build timestamp) is shown there, but this is less useful than the
version number.
So for now, just hardcode it. We're changing a translatable string
anyway.
https://phabricator.endlessm.com/T33945
https://phabricator.endlessm.com/T17239 * 2023-11-24: Merged with 753b248 “Migrate transifex configuration for use with new tx client”
Includes integration with Transifex. * 2023-11-24: Adopt upstream ka.po https://phabricator.endlessm.com/T13620 https://phabricator.endlessm.com/T24129 https://phabricator.endlessm.com/T25388 https://phabricator.endlessm.com/T28002 https://phabricator.endlessm.com/T30949 https://phabricator.endlessm.com/T33861 (cherry picked from commit 7d18a23) https://phabricator.endlessm.com/T35039
To get X-Flatpak-RenamedFrom from the .desktop file, we would like to
call g_key_file_get_string_list(), but GDesktopAppInfo does not expose
its internal GKeyFile, nor provides a get_string_list() wrapper.
GKeyFile does not expose its string-list parser, so we open-code our
own.
An interesting quirk of the GKeyFile format is that lists are (supposed
to be) ;-terminated, not just ;-delimited. This is so you can express an
empty trailing element: "a;;" represents the list ["a", ""].
g_strsplit("a;", ";", -1) returns ["a", ""], so we take care to skip any
empty elements in the returned list.
It is possible to escape a semicolon in a list: "a\;b;" represents
["a;",
"b"]. We cannot support this here, because g_key_file_get_string(), used
internally by g_desktop_app_info_get_string(), rejects the "\;" sequence
and returns NULL.
In practice, .desktop filenames do not contain semicolons (or indeed any
characters more exotic than ASCII alphanumerics, hyphens, underscores
and dots) so this code will be fine. (The alternative would be to
re-parse every .desktop file using GKeyFile directly, fish this field
out, and throw it away again.)
https://phabricator.endlessm.com/T22596
AllView monitors ShellAppSystem::installed-changed, which indicates that 1 or more .desktop files have been added, removed, or changed, and queues a call to _redisplay(). Since no 'force' parameter is passed to BaseAppView._redisplay() in this case, it calls BaseAppView.iconsNeedRedraw() to decide whether anything has actually changed. That method checks for any (re)moved items, and if there are none, compares the old and new versions of each item. However, it previously only compared their name and their icon (name). This meant that changes to (for example) the Exec= line of a .desktop file would not be picked up until something else (such as a logout and login) causes the grid to be refreshed. In the case of a backwards-incompatible change to the application's command-line arguments, this would break launching the updated app from the icon grid. 2019-10-03: adapt checks to new upstream code https://phabricator.endlessm.com/T23877
In the upstream version of this file there is only one instance of this code. We have two, in app_is_stale() and app_info_changed(). The order of comparisons is different between the two, but ignoring that, the only difference is that the latter called g_app_info_equal(). For GDesktopAppInfo, g_app_info_equal() just compares the two desktop file IDs; in the common case, these are derived from the on-disk path, so comparing g_desktop_app_info_get_filename(), as app_is_stale() did, is enough. But, g_desktop_app_info_get_filename() can return NULL if the object was constructed with g_desktop_app_info_new_from_keyfile(); in that case, g_app_info_equal() falls back to pointer equality. With this patch, we call g_app_info_equal() on both code paths, and compare the results of g_desktop_app_info_get_filename() with g_strcmp0() since they can be NULL. https://phabricator.endlessm.com/T23877
Previously, we handled renamed apps on the icon grid, but not on the taskbar. There is some clear overlap with the RENAMED_DESKTOP_IDS in this file, which is inherited from upstream; https://gitlab.gnome.org/GNOME/gnome-shell/issues/593 filed to discuss upstreaming our approach. https://phabricator.endlessm.com/T22596
T35065 handle app renaming
This will be used by the PAYG support to determine the available time remaining for activations. https://phabricator.endlessm.com/T30647
This includes the PAYG manager component (used to interface with the PAYG daemon) as well as helper classes and custom widgets to be used by the PAYG lockscreen and status applet. * UI support for backends with different code lengths: (T33325) Make use of the exposed CodeLength D-Bus property on eos-paygd's interface to inform the user how many characters should be entered. Original code imported from Endless 3.8 gnome-shell. https://phabricator.endlessm.com/T30647 https://phabricator.endlessm.com/T32864 https://phabricator.endlessm.com/T31749
This will be shown underneath the screenshield when the machine has PAYG enabled and no valid code is currently active, right before showing the screen to unlock the user's session, if a password has been set. * Port PaygUnlockDialog to new MetaIdleAdd API (T33021) Original code imported from Endless 3.8 gnome-shell. https://phabricator.endlessm.com/T30647 https://phabricator.endlessm.com/T32864 https://phabricator.endlessm.com/T31749 https://phabricator.endlessm.com/T33325
Original code imported from Endless 3.8 gnome-shell. https://phabricator.endlessm.com/T30647
Original code imported from Endless 3.8 gnome-shell. https://phabricator.endlessm.com/T30647
This indicator shows the time remaining for an active PAYG account and relevant actions. The indicator hides if this system is not set up for PAYG. Original code imported from Endless 3.8 gnome-shell. https://phabricator.endlessm.com/T30647
Original code imported from Endless 3.8 gnome-shell. https://phabricator.endlessm.com/T30647
Endless 5.2: Add quick settings toggle displaying PAYG account details and actions, replacing the previous menu items.
Endless 5.2: Add dwellClick and quickSettings to payg unlock panel.
T35064 add payg patches
With the added set of application aliases, it is possible that BaseAppGridViewLayout._redisplay will call _moveItem for an application which is no longer present in the icon grid. This requires an unusual series of events, so for simplicity, we will simply log the exception and carry on. https://phabricator.endlessm.com/T34672
wjt
reviewed
Feb 8, 2024
Comment on lines
+1105
to
+1109
| try { | ||
| this._moveItem(icon, page, position); | ||
| } catch (error) { | ||
| log(`Error moving app icon: ${error.message}`); | ||
| } |
Member
There was a problem hiding this comment.
I'm sure this will suppress the error but it doesn't seem like the right way to handle this. If you have two apps in your app grid layout, and one becomes an alias for the other, the old name should be removed from the layout, I think.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
20 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
With the added set of application aliases, it is possible that BaseAppGridViewLayout._redisplay will call _moveItem for an application which is no longer present in the icon grid. This requires an unusual series of events, so for simplicity, we will simply log the exception and carry on.
https://phabricator.endlessm.com/T34672