Skip to content
This repository has been archived by the owner on May 10, 2018. It is now read-only.

Crash when double clicking on checkbox of a plugins! #504

Closed
srazi opened this issue Aug 13, 2012 · 6 comments
Closed

Crash when double clicking on checkbox of a plugins! #504

srazi opened this issue Aug 13, 2012 · 6 comments
Labels

Comments

@srazi
Copy link
Member

srazi commented Aug 13, 2012

1- Open "Properties->Extensions"
2- Now select plugins item on the list-view (settings of a plugins is enabled if its item is selected), with GreaseMonkey and TestPlugins it doesn't crash, try others!
3- Do a double click on checkbox or a click-click (in another words, try to open setting dialog by this way).
If it doesn't crash disable plugins and restart app. and do steps again.

@ff2000
Copy link
Contributor

ff2000 commented Aug 14, 2012

It crashes on every extension that offers settings.
The root cause is that the double click gets delivered before the itemChanged, that checks for the new checkState and unloads the plugin, while the settings dialog is still in the state of being opened.

backtrace:
#0 0x00007ffff52807a2 in QApplication::x11ProcessEvent (this=0x7fffffffb530, event=0x7fffffffb150) at kernel/qapplication_x11.cpp:3647
#1 0x00007ffff52a77aa in x11EventSourceDispatch (s=0x60c8b0, callback=0, user_data=0x0) at kernel/qguieventdispatcher_glib.cpp:146
#2 0x00007ffff19a80f3 in g_main_dispatch (context=0x608cd0) at gmain.c:2539
#3 g_main_context_dispatch (context=0x608cd0) at gmain.c:3075
#4 0x00007ffff19a8440 in g_main_context_iterate (dispatch=1, block=, context=0x608cd0, self=) at gmain.c:3146
#5 g_main_context_iterate (context=0x608cd0, block=, dispatch=1, self=) at gmain.c:3083
#6 0x00007ffff19a8504 in g_main_context_iteration (context=0x608cd0, may_block=1) at gmain.c:3207
#7 0x00007ffff4d074af in QEventDispatcherGlib::processEvents (this=0x60a390, flags=...) at kernel/qeventdispatcher_glib.cpp:424
#8 0x00007ffff52a744e in QGuiEventDispatcherGlib::processEvents (this=, flags=...) at kernel/qguieventdispatcher_glib.cpp:204
#9 0x00007ffff4cd7bb2 in QEventLoop::processEvents (this=, flags=...) at kernel/qeventloop.cpp:149
#10 0x00007ffff4cd7e07 in QEventLoop::exec (this=0x7fffffffb4e0, flags=...) at kernel/qeventloop.cpp:204
#11 0x00007ffff4cdcaf5 in QCoreApplication::exec () at kernel/qcoreapplication.cpp:1187
#12 0x0000000000402c4a in main (argc=1, argv=) at main.cpp:135

(sidenote: IMHO the bt generated and saved by qupzilla is not that useful because it only shows addresses instead of

Solutions?

  • disable openSettings on double click
  • unload plugins only when saving the settings/closing the settings dialog

@nowrep nowrep closed this as completed in 2458cb8 Aug 14, 2012
@xmt
Copy link
Contributor

xmt commented Sep 7, 2012

Simply disabling the double click is not tackling the root of the problem here. This bug can still be reproduced by opening the settings dialog for a plugin, then unchecking it in the list while the settings window is still open.

This works with the Access Keys Navigation, Mouse Gestures, and PIM plugins because their settings dialog aren't modal.

I think delayed unloading as suggested by ff2000 is the way to go here, as it doesn't compromise the existing functionality.

@ff2000
Copy link
Contributor

ff2000 commented Sep 10, 2012

Could you try (and test)
https://github.com/ff2000/qupzilla/tree/pluginmanager_settings_fix

It catches ChildAdd/ChildRemove-events - which hopefully occur only when the settings dialog pops up and closes.

@nowrep
Copy link
Member

nowrep commented Sep 10, 2012

It does not fix it. Plugins should close all created widgets on unload instead.

@ff2000
Copy link
Contributor

ff2000 commented Sep 10, 2012

Ah - the "allow application extensions" checkbox was still clickable.
-> updated in the branch.

But have you been able to uncheck one of the list items?
Have there been other quirks? Because I think just not allowing to click into the listView + only allowing one settings widget at a time makes the whole thing more usable. Because without this patch it is possible to open an arbitrary number of settings widgets.

@nowrep
Copy link
Member

nowrep commented Sep 10, 2012

Yeah, and that should also be the plugin's responsibility to allow opening only one settings widget. I am going to rewrite it now.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

4 participants