-
-
Notifications
You must be signed in to change notification settings - Fork 50
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update extension.js #181
base: master
Are you sure you want to change the base?
Update extension.js #181
Conversation
@@ -245,9 +247,25 @@ class ClipboardIndicator extends PanelMenu.Button { | |||
this.menu.actor.connect('key-press-event', (_, event) => | |||
this._handleGlobalKeyEvent(event), | |||
); | |||
|
|||
this._settingsChangedId = this.settings.connect( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are these changes necessary? I believe they'll cause a crash if fields like entries
haven't been initialized yet.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a race condition that occurs if entries disappear before Store.buildClipboardStateFromLog
is finished.
this.searchEntry.get_clutter_text()
segfaults since the searchEntry
UI element no longer exists in the shell when locking the screen.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes but isn't the point of the isDestroying variable to avoid calling that code? Why do we need both?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isDestroying
wasn't enough in my testing, I still got segfaults using that variable
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Huh, well that's annoying. Do you know why? To me that means this code isn't actually related because I don't see how it could possible be run if you have the destroying flag is set.
I'd really like to know why the flag doesn't work, but we can also flip the problem around. Add an _isLoaded
variable that we set to true at the end of buildClipboardStateFromLog
and then check for it in each of the callbacks. _setupSelectionChangeListener
will be annoying cuz you'll have to add the if check inside the relevant callbacks initialized by that method.
Fixes #180