Re: Review Request 124375: Fix potential endless recursion in PanelView::event() handler
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124375/#review82604 --- Ship it! Ship It! - David Edmundson On July 16, 2015, 1:48 p.m., Daniel Vrátil wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124375/ --- (Updated July 16, 2015, 1:48 p.m.) Review request for Plasma. Repository: plasma-workspace Description --- `PanelView::event()` generates new events for itself and dispatches them via `QCoreApplication::sendEvent()`, which calls the handler directly. This can lead to endless recursion. This patch changes the handler to use `QCoreApplication::postEvent()` instead to enqueue the new event and dispatch it from `QEventLoop` after the current `PanelView::event()` returns. Downstream bug report: https://bugzilla.redhat.com/show_bug.cgi?id=1226644 Diffs - shell/panelview.cpp b94673d Diff: https://git.reviewboard.kde.org/r/124375/diff/ Testing --- Thanks, Daniel Vrátil ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 124375: Fix potential endless recursion in PanelView::event() handler
On July 16, 2015, 4:34 p.m., David Edmundson wrote: Surely all this patch will do is change it from infinitely recursing to 100% CPU busy looping in each event loop? I don't see how that's much better? With the event handling going through the event loop there is a high chance that you eventually move the mouse to a place that the code stops emitting the event further. With recursive event handling you get stack overflow and crash sooner or later. So yes, it's not perfect, but it solves a potential crash. - Daniel --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124375/#review82572 --- On July 16, 2015, 3:48 p.m., Daniel Vrátil wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124375/ --- (Updated July 16, 2015, 3:48 p.m.) Review request for Plasma. Repository: plasma-workspace Description --- `PanelView::event()` generates new events for itself and dispatches them via `QCoreApplication::sendEvent()`, which calls the handler directly. This can lead to endless recursion. This patch changes the handler to use `QCoreApplication::postEvent()` instead to enqueue the new event and dispatch it from `QEventLoop` after the current `PanelView::event()` returns. Downstream bug report: https://bugzilla.redhat.com/show_bug.cgi?id=1226644 Diffs - shell/panelview.cpp b94673d Diff: https://git.reviewboard.kde.org/r/124375/diff/ Testing --- Thanks, Daniel Vrátil ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: baloo monitor visibility
It seems this desktop search monitor fits squarely into the powerful when needed bucket. In that sense I don't think it is necessary to expose it through any of the primary search interfaces (Alt-F2 or application menu). That doesn't mean it should be completely hidden away since when it is actually needed, it shouldn't be difficult to find. I'm thinking perhaps we fold it into Search kcm as an indication of the status of the search system. It could be helpful there if the user wants to make a change to the search settings based on that information. I think we should still do the plasmoid for the instances where the user might like to do some long term monitoring. I'd suggest providing a way to access the Search kcm from the plasmoid in case that user would like to make some changes based on what they're seeing. That's my 2c. Hope it helps, Andrew On Fri, Jul 17, 2015 at 1:33 AM Harald Sitter sit...@kde.org wrote: On Wed, Jul 15, 2015 at 10:39 AM, Pinak Ahuja pinak.ah...@gmail.com wrote: In fact I'm working on a plasmoid with the monitor's functionality, which will show up in the tray. Maybe we can hide the monitor from applications and add a button to plasmoid? i.e. if we do end up shipping the plasmoid. I think that would give it much more context, so that'd be cool. Mark and Martin do have a point in that it potentially being valuable without context though (such as when run from krunner by someone who knows what it does). I am not sure having it completely standalone is necessarily the way to go about this though. In particular I'd much rather have it be a KCM in ksysguard as it is a diagnostic tool for all intents and purposes (or kinfocenter... or fold it into the regular baloo KCM?). The one and only problem I have with the tool being in the menu is that it lacks any sort of context and at least to me file search is part of the workspace at large, so it being listed as a utility feels a lot like if for example the plasma wallpaper configuration was listed there. Perhaps you could ask the visual design group for some input on this? HS ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
