> On Sept. 25, 2012, 9:03 p.m., Frank Reininghaus wrote: > > Thanks for analysing this issue and for the patch! > > > > We should of course let those events propagate. However, I'm a bit > > confused. The docs of QObject::eventFilter() don't mention anything about > > setting the 'accepted' state of the event. If I understand it correctly, > > the event is filtered (i.e., not handled further) only if eventFilter() > > returns true. > > > > Even if your patch fixes the issue, I'm therefore wondering if it just > > hides the real bug. > > > > I think that what is actually supposed to happen is that DolphinView does > > not filter the event and that the KItemListView does not handle the event > > either, such that the unhandled event is passed through to the parent > > widgets until it ends up in the KonqCombo, such that > > KonqMainWindow::eventFilter() gets it, right? If that is true, I think the > > real bug might be that some class pretends to handle the event, such that > > its parents don't see it, although it does nothing with the event. > > > > I had a quick look at the code and found that KItemListView::event() > > accepts the event and returns true if m_controller->processEvent() returns > > true, but does not ignore() the event if that is not the case. I haven't > > tested it yet, but maybe that is the real problem? > > Thomas Lübking wrote: > Accepting the event means that it's not further propagated (to the > parenting widget) > returning "true" in the event filter means to "eat" the event, ie. > prevent regular handling in following eventfilters or even the widget event > handling itself. > > The patch will ensure that the (input, i assume) event goes up to the > parent widget and is handled there if nothing else accepts it. > If you returned true, no further processing would happen at all (ie. even > DolphinView::event() would process it) > > You should btw. ensure that the parenting QScrollArea accepts it, or > oxygen will get bugs because selecting items in dolphin will start to move > the window ;-) > > Disclaimer: i've not even seen the bug, just generally commented on > question and the one-liner diff
PS: the actually more correct fix was to align to the Qt::WA_NoMousePropagation attribute and set it accordingly (allowing the usercode to manage the behavior this way) - Thomas ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106569/#review19423 ----------------------------------------------------------- On Sept. 25, 2012, 3:38 p.m., Dawit Alemayehu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/106569/ > ----------------------------------------------------------- > > (Updated Sept. 25, 2012, 3:38 p.m.) > > > Review request for Dolphin and KDE Base Apps. > > > Description > ------- > > This patch fixes a bug where pressing CTRL+Tab does not switch tabs when > active tab in Konqueror is a Dolphin's filemanagement part. It happens > because DolphinView installs an event filter and does not call > event->ignore() or event->setAccepted(false) to allow those events to be > propagated up the event chain. > > > This addresses bug 302329. > http://bugs.kde.org/show_bug.cgi?id=302329 > > > Diffs > ----- > > dolphin/src/views/dolphinview.cpp 0584972 > > Diff: http://git.reviewboard.kde.org/r/106569/diff/ > > > Testing > ------- > > > Thanks, > > Dawit Alemayehu > >
