When using dynamic workspaces, it is possible to try to change to a
non-existent one if the user defines hotkeys for changing to desktop
1, 2, 3... This case is not detected, and gnome shell shows an error:
JS ERROR: TypeError: workspace is null
actionMoveWorkspace@resource:///org/gnome/shell/ui/windowManager.js:2130:13
wrapper@resource:///org/gnome/gjs/modules/_legacy.js:82:22
_showWorkspaceSwitcher@resource:///org/gnome/shell/ui/windowManager.js:2104:13
wrapper@resource:///org/gnome/gjs/modules/_legacy.js:82:22
This patch adds a check before trying to change the workspace, to avoid
switching to a non-existent one.
https://gitlab.gnome.org/GNOME/gnome-shell/merge_requests/365
When an InputSourceIndicator is destroyed, the InputSourceManager it was
connected to could (and probably will) outlive it (since the manager is
a singleton). If the InputSourceManager emits any subsequent signals,
the callbacks from the finalised InputSourceIndicator could be invoked,
and will reference finalised objects.
This can be triggered by running `pkexec true` from a gnome-terminal
window, then calling `pkill pkexec` from another terminal (on a
different VT or via SSH). This causes the dialogue to be cancelled by
polkitd.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
https://gitlab.gnome.org/GNOME/gnome-shell/merge_requests/357
Otherwise the user object could outlive the dialogue, emit a subsequent
signal, and the callback from that signal could reference finalised
objects/widgets from the dialogue. The likely mechanism for the user
outliving the dialogue is caching of user objects within
libaccountsservice.
This can be triggered by running `pkexec true` from a gnome-terminal
window, then calling `pkill pkexec` from another terminal (on a
different VT or via SSH). This causes the dialogue to be cancelled by
polkitd.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
https://gitlab.gnome.org/GNOME/gnome-shell/merge_requests/357
Rather than explicitly destroying the session after calling close(),
destroy it from the `closed` signal handler.
This also means we can make the method internal.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
https://gitlab.gnome.org/GNOME/gnome-shell/merge_requests/357
In case there are any internal ways the dialogue can close itself
without calling its own close() method, it’s probably better to do all
our cleanup on a handler for the `closed` signal instead.
This should introduce no functional changes except ensuring the
polkitAgent cleanup is always done.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
https://gitlab.gnome.org/GNOME/gnome-shell/merge_requests/357
Otherwise the session could outlive the dialogue, emit a subsequent
signal, and its callback would reference finalised objects/widgets from
the dialogue. The PolkitSession object is implemented by
libpolkit-gobject, so we have no guarantees about its reference counting
— the session object could keep itself alive in another thread, or be a
singleton. In all likelihood, the session hangs around for longer than
the dialogue due to differences in when the two objects are garbage
collected.
This can be triggered by running `pkexec true` from a gnome-terminal
window, then calling `pkill pkexec` from another terminal (on a
different VT or via SSH). This causes the dialogue to be cancelled by
polkitd.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
https://gitlab.gnome.org/GNOME/gnome-shell/merge_requests/357
This avoids the following warning sometimes happening later:
JS WARNING: [resource:///org/gnome/shell/ui/search.js 701]: reference to undefined property "searchInProgress"
Signed-off-by: Philip Withnall <withnall@endlessm.com>
https://gitlab.gnome.org/GNOME/gnome-shell/merge_requests/357
The built-in calendar isn't meant to replace a full-fledged calendar
app, which is why clearing event messages only hides the event in
gnome-shell rather than deleting the actual event. This has turned out
to not be overly useful and often confusing - it creates a discrepancy
with visible events in apps, isn't revertible in a non-obscure fashion
and non-obviously limited to the current date.
As we are considering moving events out of the message list and back to
the calendar, it looks like a good time to remove that ability and keep
notifications as the only removable messages.
https://gitlab.gnome.org/GNOME/gnome-shell/issues/262
We currently deliberately avoid chaining up in derived policy
constructors to not override properties with their defaults.
That's a neat trick that will stop working when porting to ES6
classes, as chaining up is necessary to actually initialize the
object there (including "this").
Address this by turning all properties into (overridable) getters
that are backed by private properties by default.
https://gitlab.gnome.org/GNOME/gnome-shell/merge_requests/350
The _createPolicy() method of a subclass usually depends on some
constructor parameters that need to be set before chaining up to
the parent. This works fine with Lang.Class, but will break with
ES6 classes, as "this" is only initialized after chaining up.
Prepare for this by not creating the policy in the constructor,
but when it is first accessed.
https://gitlab.gnome.org/GNOME/gnome-shell/merge_requests/350
Check via Polkit if the current user is actually allowed to enroll
devices before trying to do so. If not, show a notification that
explains that a system administrator needs to authorize the device.
Clicking on the notification will guide the user to the thunderbolt
control center panel. Before this patch, when the current user was
not allowed to enroll a device a polkit dialog would pop up which
is confusing because it did not contain any information why it was
shown. This patch implements the behavior as designed (see [1],
section "Multi-user environments").
[1] https://wiki.gnome.org/Design/Whiteboards/ThunderboltAccess
Since commit 5fb8d4f730, a NotificationMessage's notification property
is reset to null when the notification is destroyed. However at that
point we still have connected signal handlers around that we'll try
to disconnect later.
Avoid the warnings by disconnecting and resetting the handler IDs at
the same time as the notification.
https://gitlab.gnome.org/GNOME/gnome-shell/merge_requests/308
In contrast to generic animated icons, it is reasonable to expect
spinners to be invisible while inactive. Implement that behavior
in the new Spinner class and optionally animate the transitions.
https://gitlab.gnome.org/GNOME/gnome-shell/merge_requests/316
If the session mode doesn't allow access to Settings, the language
menu should respect that and not expose the "Region & Languages"
panel. Using the dedicated method instead of manually constructing
the menu item takes care of that and makes for less code.
https://gitlab.gnome.org/GNOME/gnome-shell/issues/780
The picked target actor may be destroyed (e.g. hover style change
resulting in the ClutterTexture to be destroyed). If we don't handle
this, GJS will abort when it sees the exception caused by Javascript
code trying to access the destroyed target actor.
To handle it, listen on the 'destroy' signal on the target actor, and
repick, so a valid actor is passed to the next motion callback.
Fixes: https://gitlab.gnome.org/GNOME/gnome-shell/issues/632
When a fullscreen application is focused,
toggling the overview via hot-corner is disabled,
even when the overview is currently visible.
This only makes sense, when the overview is
hidden to not to disturb the behaviour of the
fullscreen application, but leaves an
inconsistency when the overview is visible since
it should work there like when a non-fullscreen-
application is focused.
So, always allow hiding the overview using the
hot corner when the overview is visible.
https://gitlab.gnome.org/GNOME/gnome-shell/issues/429
Add a D-Bus API that allows the API user to introspect the application
state of the shell. Currently the only exposed information is list of
running applications and which one is active (i.e. has focus).
https://gitlab.gnome.org/GNOME/gnome-shell/merge_requests/326
The intention of commit 4dc20398 was to disable unredirection while
banners are shown, but the ::done-displaying signal currently used for
re-enabling unredirection is only emitted under some circumstances, so
it's possible that unredirection is left disabled indefinitely, whoops.
Fix this by tying disabling unredirection explicitly to the lifetime
of the banner actor.
https://gitlab.gnome.org/GNOME/gnome-shell/issues/799
The only reason for `vfunc_paint` and `vfunc_pick` existing was to
implement a culling optimization. Although that optimization actually
made performance worse than none at all because it forced the painting
and picking cycles to spend more time calling into JavaScript.
Turns out we don't have to choose between native code and culling though.
Just reimplement the culling using native ClutterActor functions and we
get the benefits of both.
Performance on an i7-7700:
Moving the cursor over the icon grid:
Before: 70% CPU, 5.5ms per frame
After : 60% CPU, 4.5ms per frame
Scrolling the icon grid:
Before: 60% CPU, 4.4ms per frame
After : 50% CPU, 3.3ms per frame
Helps with https://gitlab.gnome.org/GNOME/gnome-shell/issues/174
Destroy the DashItemContainer's child from the same handler as the tooltip. This
will prevent invalid reads when the item is destroyed while its quicklist is
still open.
Fixes: https://gitlab.gnome.org/GNOME/gnome-shell/issues/781
Back in the day, there was a proposed system of tracking apps in a
specific context.
The inspiration was that you may have used apps in multiple modes:
Firefox may have been used in both "Programmer Reference" and
"Kitten Videos" contexts. Early user response to the feedback wasn't
too positive - context switching is something that humans have trouble
doing implicitly, let alone explicitly. The old codebase still has a
few remnants of this around; let's finally put them to rest.
Note that we still write out a dummy context tag to the XML file - old
versions of the shell will flat out crash if you don't have one of those
in there, so just leave it in for compatibility sake.
https://bugzilla.gnome.org/show_bug.cgi?id=673767
The pending-charge state means AC power is on but the battery is not
being charged. This can happen because its charge is above a certain
threshold, to avoid short charging cycles and prolong the battery's
life, or because the PSU is not powerful enough to charge the batteries.
Instead of lying to the user about something being estimated, we should
simply tell the truth and set the label to "Not Charging".
Closes: #701.
When we started to only show a single caption at a time, we allowed
title captions to be wider than their corresponding window preview.
But while overlapping neighboring previews is fine, we shouldn't
allow the captions to leak outside the workspace area itself and
overlap unrelated elements like workspace switcher or dash.
This partly reverts commit b3b30f239d.
https://gitlab.gnome.org/GNOME/gnome-shell/merge_requests/214
Instead of defaulting to a natural scroll behavior,
have the workspace switch action use the natural-scroll setting
in org.gnome.peripherals.touchpad to determine the correct
direction of travel when swiping. 4 finger swipes will then
match the behavior of the rest of the UI.
Reference: https://gitlab.gnome.org/GNOME/gnome-shell/issues/516
While this sounds counter-intuitive, the image-path hint value might also
be used with URIs or icon names.
As per freedesktop standard:
The "app_icon" parameter and "image-path" hint should be either an URI
(file:// is the only URI schema supported right now) or a name in a
freedesktop.org-compliant icon theme (not a GTK+ stock ID).
Thus the image-path hint should also be parsed as it happens for the
app_icon.
Reuse same logic, by falling back on _iconForNotificationData with the
hint value.
https://gitlab.gnome.org/GNOME/gnome-shell/merge_requests/285
We have a callback that will call close() when the notification is
destroyed, and a callback that will call destroy() on the notification
when the message is closed.
Currently, if the notification is destroyed we'll execute our callback
that will call again destroy() on the notification. That's bad
practice in general, and it also has the side effect of resetting the
destroy reason.
This commit avoids re-destroying the notification by dropping the
notification reference on destroy.
https://gitlab.gnome.org/GNOME/gnome-shell/merge_requests/258
Differently from the fd.o notifications, Gtk notifications do not
have a mechanism to update themselves. Instead, when a new
notification is received for an ID already known to the notification
daemon, the old notification is dismissed and a replaced with a new
one.
Currently though, there is no way to distinguish a notification that
was dismissed because of an user interaction, or because it was
replaced. That is an useful piece of information, so add a new value
to the NotificationDestroyedReason enum to account for it.
https://gitlab.gnome.org/GNOME/gnome-shell/merge_requests/258
When gnome-shell receives the signal of 'set-content-type' from ibus,
gnome-shell calls KeyboardManager.holdKeyboard() and
KeyboardManager.releaseKeyboard() and the functions change the current
input focus in GNOME Xorg and it could result in closing a popup window
which has a password entry by focusing on the entry.
The solution is to stop to call the APIs on 'set-content-type' signal.
https://gitlab.gnome.org/GNOME/gnome-shell/issues/391
We don't usually show notification banners while the monitor is in
fullscreen, but when we do - the notification is urgent - we should
actually show the banner, even if the top-most window is unredirected.
To achieve that, disable unredirection while the banner is showing.
https://gitlab.gnome.org/GNOME/gnome-shell/issues/430
The `reactive` property of icon actors was being restored multiple times
over the course of the pulse animation, all at slightly different times
as each icon finished animating at different times.
The problem is that toggling `reactive` on an `StWidget` incurs a style
change of the `insensitive` pseudo class, and style changes would quickly
queue relayouts incurring full stage reallocation. This occurred many times
during a pulse animation, limiting its smoothness and performance.
The solution is to not toggle the `reactive` property in the pulse
animation at all, which avoids incurring multiple full stage relayouts.
As a bonus, this means the icon under the cursor pulses with the correct
selection highlight, appearing more seamless and responsive.
https://gitlab.gnome.org/GNOME/gnome-shell/merge_requests/261
The `reactive` property of icon actors was being restored 24 times over
the course of the spring animation, all at slightly different times as
each icon finished animating at different times.
The problem is that toggling `reactive` on an `StWidget` incurs a style
change of the `insensitive` pseudo class, and style changes would quickly
queue relayouts incurring full stage reallocation. This occurred many times
during a spring animation hogging the CPU and limiting the frame rate.
The solution is defer and batch the cleanup for all icons until after the
last icon has finished animating. This way the CPU impact of the style
change and stage relayout isn't felt during the animation so the frame
rate remains higher and smoother. The overall CPU usage of the animation
is also reduced as the remaining relayouts are much more likely to be
grouped into a single frame.
Icon spring animation performance on an i7-7700:
Before: 83% CPU and 47 FPS
After : 78% CPU and 54 FPS
which is about a 22% increase in performance per clock (FPS/CPU).
https://gitlab.gnome.org/GNOME/gnome-shell/merge_requests/253
The switcher popup is a large, mostly transparent actor that
should cover all the clickable area of GNOME Shell. In Clutter
terms, it should cover the whole stage.
By binding it to the primary monitor, the Alt+Tab behavior
becomes a bit inconsistent. For example, by not hiding when
clicking at empty spaces at other monitors.
Fix that by binding the SwitcherPopup to the whole stage,
and not only the primary monitor.
Fixes https://gitlab.gnome.org/GNOME/gnome-shell/issues/647
If no password or a wrong password is entered after automounting an
encrypted device, then the password should be reasked. However, this
does not happen because the relevant udisks error messages for this
cases are missing in the exception handler that calls _reaskPassword.
Fix this issue by adding the relevant udisks error strings to the
exception handling in the _onVolumeMounted method.
https://gitlab.gnome.org/GNOME/gnome-shell/issues/640
We currently only ignore minimized windows, not windows that are
hidden for other reasons - namely on wayland windows are initially
hidden until they are placed.
This fixes a flicker in the transparent top bar on wayland when the
"position" of an unplaced window wrongly suggests the window is
overlapping the top bar.
https://gitlab.gnome.org/GNOME/gnome-shell/issues/693
Since we started to show OSD windows on all monitors, OSD windows are
destroyed when the corresponding monitor is disconnected. We shouldn't
leave any signal handlers around in that case - they prevent the object
from being garbage collected, and trigger warnings for accessing proper-
ties of invalidated GObjects.
https://gitlab.gnome.org/GNOME/gnome-shell/issues/602
When maxLevel is > 100%, first OSD appearance was capping the current
level to 100%. Consecutives key press were then OK.
Ensure we setMaxLevel before setting Level itself, so that correct cap
value is applied.
App folder popups take a grab when opened, and as we don't pass any
particular pushModal() parameters, all keybindings are blocked. While
this makes sense for most keybindings that would interfere with the
popup interaction, others like volume/brightness keys or screenshots
can be allowed safely.
https://gitlab.gnome.org/GNOME/gnome-shell/issues/648
Add exception to handle a keypress if numlock is enabled as we already do for
capslock. This uses Clutter.ModifierType.MOD2_MASK because at the moment there
is not a more explicit way to refer to the numlock mask.
Fixes: https://gitlab.gnome.org/GNOME/gnome-shell/issues/550
Instead of taking care of the PanelMenu.ButtonBox.container
destruction by itself, delegate that to the very object that
created it in the first place: PanelMenu.ButtonBox itself.
This is the last remaining usage of Shell.GenericContainer
in the codebase, and posed small challenges compared to the
other removals.
A new St.Widget subclass called InputSourceIndicatorContainer
was added as a replacement to the Shell.GenericContainer. It
was needed because GNOME Shell needs to override the regular
size allocation functions, but InputSourceIndicator already
is a St.Widget with its own size allocation overrides.
https://gitlab.gnome.org/GNOME/gnome-shell/merge_requests/153
Because we're late in the cycle, and don't know how many
extensions actually rely on this API, this commit adds
back the BoxPointer.show() and .hide() functions, with
warning messages to notify consumers that this is going
to be removed.
https://gitlab.gnome.org/GNOME/gnome-shell/merge_requests/153
Pretty much like dd4709bb2, BoxPointer's show() and hide()
functions will clash with Clutter.Actor's ones.
In addition to that, on a conceptual level, the current API
is not great, because calling boxPointer.hide() won't result
in boxPointer.actor.visible == false.
For these reasons, rename show() and hide() to open() and
close(). A compatibility layer will be added in a following
commit, warning about the usage of show() and hide().
https://gitlab.gnome.org/GNOME/gnome-shell/merge_requests/153
LayoutManager is currently a pure JavaScript class that
relies on the rudimentary Signals.addSignalMethods() to
handle signals. This is an inefficient implementation of
one of the most central classes in GNOME Shell.
In addition to removing Shell.GenericContainer, then,
turn LayoutManager into a proper GObject.Object subclass.
https://gitlab.gnome.org/GNOME/gnome-shell/merge_requests/153
In the next commit, we will turn PanelMenu.ButtonBox into a
St.Widget subclass. As a domino effect, PanelMenu.Button will
become one too, and so will Panel.AppMenuButton.
When that happens, the current show() and hide() functions in
Panel.AppMenuButton will clash with Clutter.Actor's ones.
To avoid that, rename these functions to fadeIn() and fadeOut()
and avoid a name clash.
https://gitlab.gnome.org/GNOME/gnome-shell/merge_requests/153
This is another straight port from Shell.GenericContainer.
The important thing to notice is that the calculation is
broken if the StThemeNode helpers (adjust_preferred_* and
adjust_for_*) aren't used.
The downside of this patch is that it removed the skip_paint
from the thumbnails. Keeping it would add an unecessarily
large amount of code.
https://gitlab.gnome.org/GNOME/gnome-shell/merge_requests/153
Removing Shell.GenericContainer from the IconGrid class was
challenging because it needs the "skip paint" API from it.
This API was added, too, as a workaround to the inability
to override vfuncs from GJS.
The overrides are largely copy-pasted and translated versions
of the Shell.GenericContainer code.
The IconGrid:key-focus-in signal was renamed to :child-focused
to avoid clashing with ClutterActor:key-focus-in.
In GridSearchResults, the internal IconGrid had it's y_expand
set to false, so it doesn't push other search elements (the
list results mainly) to the bottom of the screen.
Because skip paint wasn't and still isn't a GObject property,
rename it to _skipPaint to reflect that.
https://gitlab.gnome.org/GNOME/gnome-shell/merge_requests/153
As part of our quest to obsolete Shell.GenericContainer, IconGrid will
become a Clutter.Actor subclass. As the ::key-focus-in signal would
clash with Clutter.Actor::key-focus-in, rename it to ::child-focused.
https://gitlab.gnome.org/GNOME/gnome-shell/merge_requests/153
DashItemContainer currently animates the scale and opacity
of its child when zooming in. This is visible when adding
a new favorite item to the dash; the items will zoom in from
the center.
After the previous commit, however, the zoom animation got
slightly broken, and looked like the icon was coming from
the bottom instead of the center.
Fix that by setting the scale and opacity of DashItemContainer
itself, instead of its child. Remove the unused code after that
too.
https://gitlab.gnome.org/GNOME/gnome-shell/merge_requests/153
Pretty much like the previous patches, this extends St.Bin. The
most interesting aspect of this patch is that most of the sizing
routines of the icons is now delegated to the actors and layout
managers, removing quite a bunch of code.
The 'spacing' theme property is now redirected to StBoxLayout's
spacing property. Also adjust the Dash code to stop forcing a
potentially invalid width in the first icon too.
https://gitlab.gnome.org/GNOME/gnome-shell/merge_requests/153
This commit removes all the uses of Shell.GenericContainer from
SwitcherPopup.SwitcherList. Compared to the other patches, this
one was specially trickier to get right, and a few invasive
changes needed to be done.
The most noticeable one is that the allocation of the items is
done entirely by St.BoxLayout -- we don't manually allocate them
anymore. To make it work, get_preferred_width() had to calculate
the correct value. It now assumes that:
* Minimum width: the minimum width of the widest child.
* Natural width: the minimum width of the StBoxLayout (use it
instead of the natural width to force the labels to ellipsize
when too long.)
The AppIcon class became a St.Widget subclass as well, to override
get_preferred_width() and be able to keep the squared shape.
Besides that, add a new SwitcherButton class to reimplement squared
icons without having to resort to hacks in the size allocation
machinery. This class has a single vfunc override to ensure that it
is squared when the SwitcherList is.
The arrows indicating multiple windows are now in this._list
actor to the SwitcherPopup itself, since this._list automatically
manages its own children now.
At last, adapt (but preserve) the hack in CyclerPopup.
https://gitlab.gnome.org/GNOME/gnome-shell/merge_requests/153
Instead of overriding vfunc_get_preferred_width|height(), use the
already available Layout.MonitorConstraint to bind SwitcherPopup
to the primary monitor.
This commit turns SwitcherPopup.SwitcherPopup into a St.Widget
subclass, and gets rid of Shell.GenericContainer usage. Subclasses
were adapted to that too.
This class introduced a new challenge: it overrides show(). As per
discussions, we now call this.visible = true inside show().
https://gitlab.gnome.org/GNOME/gnome-shell/merge_requests/153
In the process of purging all usages of Shell.GenericContainer
of GNOME Shell, one specific problematic situation that might
occur is when classes have functions that would clash with any
ClutterActor or StWidget function name.
One of such example is SwitcherPopup.destroy(). Right now, this
class is a pure JavaScript class that wraps a real actor, but
soon this will change, and it'll become a St.Widget subclass.
Another problem with functions that mimic the toolkit ones is
the predictability of them; after calling destroy(), that widget
is expected to not be available anymore. In SwitcherPopup case,
it is still available for a short while. In this case, that's not
a big problem, but the show() and hide() functions in other clases
are more problematic because the actor's visibility does not
follow that.
This commit is a first step in cleaning that up, and changes the
SwitcherPopup.destroy() to fadeAndDestroy().
https://gitlab.gnome.org/GNOME/gnome-shell/merge_requests/153
Shell.GenericContainer exposes the size negotiation machinery
through the use of signals. Signals are not specially performant.
One of the reasons is that they acquire a global lock for signal
handlers lookup. GNOME Shell has more than 2,000 actors at any
given point in time, up to 20 levels deep in hierarchy, making
size negotiation and painting non-trivial tasks. Such a critical
section of Clutter's machinery shouldn't rely on signals
whatsoever.
Regardless of that, Shell.GenericContainer is a workaround to
a non-existing issue anymore. It shouldn't be used anyway, and
any performance improvements that removing it can potentially
yield are bonuses to it.
This commit starts this work by removing Shell.GenericContainer
usage from Panel.Panel class. The class now extends St.Widget,
and as such, it has no "this.actor" field set anymore. A couple
of places where this actor field was used are adjuste as well.
It is important to notice that we now allocate the Panel itself
inside vfunc_allocate(). This was previously done before emitting
the signal by Shell.GenericContainer.
https://gitlab.gnome.org/GNOME/gnome-shell/merge_requests/153
When adding a favorite, we add the ID to the list of favorites, save the
setting and add the new app to the favorites map. However as writing the
settings value already results in reload() to update the favorites map,
the new app is usually already in the map when we add it.
The only exception is when the ID was found in the RENAMED_DESKTOP_IDS map,
in which case we end up adding both the renamed app and the original one.
Fix this by simply relying on reload() to properly update the map, just like
we already do in _removeFavorite().
https://gitlab.gnome.org/GNOME/gnome-shell/issues/471
On X11, reactive chrome must be added to the input region in order
to work as expected. However that region works independently from
any window stacking, with the result that the unresponsive-app dialog
currently blocks all input in the "covered" area, even in windows
stacked above the unresponsive window.
The correct fix would be to track the unobscured parts of the dialog
and set the input region from that, but that's quite cumbersome. So
instead, only track chrome when the corresponding window is focused
(or the dialog itself of course).
https://gitlab.gnome.org/GNOME/gnome-shell/issues/273
As a mount operation's UI may be reused (for example after mistyping
the password), we only close the operation once the mount has finished
(successfully or with error).
We therefore need to track ongoing operations, which we currently do
by monkey-patching the corresponding volume object. However while the
underlying GVolume object indeed remains the same through-out the
operation, the JS wrapper object isn't referenced anywhere and may
thus be garbage collected, resulting in a stuck dialog.
Fix this issue by tracking active operations explicitly, so that all
involved objects are referenced until the end of the operation.
https://gitlab.gnome.org/GNOME/gnome-shell/issues/565
The `hints` and `settingName` parameters to the agent call may define
the specific list of secrets NM actually needs from the user. This
seems to have been the intended use of these two parameters but only
recently did NM with the IWD backend start to use this to request 802.1x
secrets. So if `hints` is provided, ask user for the specific secrets
listed there and don't even look at what type of EAP method is in use.
Only the three types of secrets actually in use by NM's IWD backend are
supported for now -- they happen to be the same three that
_get8021xSecrets() had already supported.
This attribute was previously only assigned in show(). hide() compares
this attribute to 0. If hide() is called before show() is first called,
the comparison would give the correct result (undefined > 0 is false)
but log a warning:
JS WARNING: [resource:///org/gnome/shell/ui/workspacesView.js 529]:
reference to undefined property "_restackedNotifyId"
Initialize this attribute in _init(), alongside _scrollEventId and
_keyPressEventId which are also used in hide().
dialogContent is set to one of the elements of the list DialogContent,
but not all of those have a checkBoxText property. When logging out (as
opposed to shutting down), this causes a warning:
JS WARNING: [resource:///org/gnome/shell/ui/endSessionDialog.js
763]: reference to undefined property "checkBoxText"
(The line number corresponds to this line in 3.28.3.)
The warning is apparently not triggered if the undefined property is
used as part of a boolean expression:
gjs> var x = {};
gjs> x.a;
typein:2:1 strict warning: reference to undefined property "a"
gjs> if (x.b) { log('oh no'); }
gjs> x.c || ''
""
_setCheckBoxLabel() just checks the truthiness of its 'text' argument,
and the empty string is false-y, so passing '' rather than undefined has
no functional effect.
In recent Fedora 29, connecting to wifi access points from the
user menu (top-right menu) does not work. Clicking the 'Connect'
button just animates it but does nothing else. The logs show an
error "JS ERROR: Error: Expected type utf8 for Argument
'specific_object' but got type 'undefined'".
Looking into this, it seems the problem is these uses of the
`path` property of an NMAccessPoint. NMAccessPoint inherits
from NMObject, and NMObject *does* have a path property:
https://developer.gnome.org/libnm/stable/NMObject.html#NMObject--path
so at first glance this seems fine. But I poked around a bit
using libnm via Python (which goes via introspection, just like
this JS code does), and found that indeed AccessPoint objects
don't seem to have a `path` property there either.
Looking at the libnm code, this actually makes sense, because
the property is marked "(skip)":
https://github.com/NetworkManager/NetworkManager/blob/master/libnm/nm-object.c#L1291
and the introspection docs suggest that means it should be left
out of introspected output:
https://wiki.gnome.org/Projects/GObjectIntrospection/Annotations#Symbol_visibility
I'm a bit concerned that this was only found recently - whereas
the change to use `.path` in gnome-shell dates from October 2017
(d71af5e5) and the property has been marked (skip) in NM since
at least 2016 - but this all seems to add up. The obvious fix is
to replace use of `.path` with `.get_path()`, which returns the
path and is *not* marked (skip) and so *is* available via
introspection. I tested that this works in Python and also did
a test build of gnome-shell with this change and installed it on
an affected system, it does seem to fix the bug.
Signed-off-by: Adam Williamson <awilliam@redhat.com>
