Skip to content
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

Fix PM button desynchronization and possible crash #1799

Merged
merged 2 commits into from
Apr 3, 2018

Conversation

b4n
Copy link
Member

@b4n b4n commented Mar 10, 2018

Fixes #1781, and replaces #1784.

See 373852c for extensive details.

b4n added 2 commits March 10, 2018 17:23
This should not be needed in normal operation, but is safer if the UI
gets out of sync for some reason.
When toggling a plugin, we temporarily set the tree store's row entry
for the plugin pointer to NULL as we destroy and reload the selected
plugin, and its pointer would be invalid in the meantime.  This results
in the filter we use to display search results to temporarily hide the
row, changing the actual number of rows and thus, depending on timing,
this will or will not change the selected row (it will when double
clicking, not when single-clicking), in a seemingly more or less random
fashion as we use a sorted model.

Finally, as we manually update the buttons visibility for the toggled
plugin (as we otherwise do only for changing selection, which should
not happen in this case -- well, most of the time as you can see), this
can lead to the buttons to be updated for a now unselected row, getting
those out of sync.

The fix here is not to actually hide rows with a NULL plugin, because
it can only happen in 2 cases, where we actually want to see it:

1. while toggling a plugin, as explained above, in which case it had to
   match the search already.
2. when there is no plugins and we want to display a "No plugins
   available" message, and the search should not affect this.

This incidentally also fix the "No plugins available" so it's actually
visible, instead of always hidden.

Fixes geany#1781.
@elextr
Copy link
Member

elextr commented Mar 10, 2018

Nice commit novel :)

@b4n
Copy link
Member Author

b4n commented Mar 10, 2018

Especially as the actual diff is changing FALSE to TRUE, which is a -4 +3 character diff :)

@lpaulsen93
Copy link
Contributor

I did a quick negative and positive test by only changing the FALSE to TRUE. I can confirm that it fixes the problem for me. Thanks 👍

Today I also noticed that without the fix sometimes even the plugin below the row I am clicking/double-clicking in gets toggled.

@kugel- kugel- added the reviewed label Apr 3, 2018
@kugel- kugel- added this to the 1.34 milestone Apr 3, 2018
@b4n b4n merged commit 373852c into geany:master Apr 3, 2018
b4n added a commit that referenced this pull request Apr 3, 2018
…click

Fix PM button desynchronization and possible crash
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants