----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/129663/#review101488 -----------------------------------------------------------
I agree that doing this in Show is far too late - and that KAcceleratorManager needs to be told, to avoid the infinite loop. However the reason for this code still holds I think, so it seems to me that it needs to be improved instead of removed. => this should be done in Polish (the event, not the language :-) ) and KAcceleratorManager::setNoAccel(tb) should prevent the recursion. (... and the no-op case (nothing to remove) shouldn't trigger anything... How come tb->setText(tb->text()) does anything? Or are you seeing this bug with CJK languages only?) - David Faure On Dec. 17, 2016, 12:23 p.m., Martin Tobias Holmedahl Sandsmark wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/129663/ > ----------------------------------------------------------- > > (Updated Dec. 17, 2016, 12:23 p.m.) > > > Review request for KDE Frameworks, David Faure and Chusslove Illich. > > > Repository: kxmlgui > > > Description > ------- > > Don't try to strip out accelerators in the KToolBar event handler. It makes > no sense to me, potentially creates an endless repaint loop and fights with > KAcceleratorManager which will constantly re-add accelerators. > > > Diffs > ----- > > src/ktoolbar.cpp 31be9b0 > > Diff: https://git.reviewboard.kde.org/r/129663/diff/ > > > Testing > ------- > > With this patch not only the control button in Dolphin has an accelerator. > > > Thanks, > > Martin Tobias Holmedahl Sandsmark > >
