[Kde-pim] Review Request: akregator doesn't need to handle system tray Quit itself, KStatusNotifierItem does all that

[Teemu Rytilahti]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/102503/#review14867
-----------------------------------------------------------

Ship it!


Thanks for the patch. Looks good to me and seems to work. Getting An unknown error (perhaps from kmail?) though, when closing within Kontact immediately after it has started, from console:
kontact(31582)/kutils (KSettings::Dispatcher) KSettings::Dispatcher::DispatcherPrivate::unregisterComponent: componentName= "kmail2" refcount= 0
kontact(31582)/libakonadi Akonadi::EntityTreeModelPrivate::fetchJobDone: Job error:  "Unknown error." for collection: 1135 
kontact(31582)/kdeui (KNotification) KNotificationManager::close: 10971


I'm also getting X errors to my console when clicking cancel, but it might be just that nvidia's driver doesn't like something:
kontact(30397)/kdeui (KNotification) KNotificationManager::close: 10755
X Error: BadWindow (invalid Window parameter) 3
  Major opcode: 20 (X_GetProperty)
  Resource id:  0x58000bd
kontact(30397)/kdeui (KNotification) KNotificationManager::close: 10760
X Error: BadWindow (invalid Window parameter) 3
  Major opcode: 20 (X_GetProperty)
  Resource id:  0x58000c5
kontact(30397)/kdeui (KNotification) KNotificationManager::close: 10765

P.S. If you don't mind, please do add a mention about quit handling to http://techbase.kde.org/Development/Tutorials/PortToKStatusNotifierItem :-)

- Teemu Rytilahti


On Sept. 1, 2011, 1:41 p.m., Jonathan Marten wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/102503/
> -----------------------------------------------------------
> 
> (Updated Sept. 1, 2011, 1:41 p.m.)
> 
> 
> Review request for KDEPIM.
> 
> 
> Description
> -------
> 
> Akregator uses KStatusNotifierItem to show its icon in the system tray.  This handles the "Quit" action in the system tray icon menu internally in KStatusNotifierItemPrivate::maybeQuit(), asking for confirmation first and then calling qApp->quit() if so.
> 
> With this being the case, there is no need for the application or part to handle the system tray "Quit" action itself. The problem in doing so is that, if "Quit" is chosen from the system tray icon but then "Cancel" is clicked in the confirmation dialogue, Akregator quits anyway because of the double (or maybe even the triple) handling of this action.
> 
> This patch removes the connecting of the system tray Quit action's triggered() signal within the Akregator main window and part, leaving its handling to KStatusNotifierItem.  This means that MainWindow::slotQuit() is redundant, but I have left it in place for the moment for binary compatibility.  It can be removed if BIC is not an issue.
> 
> 
> Diffs
> -----
> 
>   akregator/src/akregator_part.cpp 6ce7350 
>   akregator/src/mainwindow.cpp 082bbe7 
> 
> Diff: http://git.reviewboard.kde.org/r/102503/diff/
> 
> 
> Testing
> -------
> 
> Built kdepim with these changes.
> 
> File - Quit from the menu bar => Akregator quits immediately.
> 
> System tray menu - Quit, dialogue "Confirm Quit From System Tray" appears.
> Click "OK" in that => Akregator quits.
> Click "Cancel" in that => Akregator does not quit.
> 
> 
> Thanks,
> 
> Jonathan Marten
> 
>

_______________________________________________
KDE PIM mailing list kde-pim at kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
KDE PIM home page at http://pim.kde.org/