Add option to discourage reporting bugs
ClosedPublic

Authored by broulik on Aug 19 2019, 1:25 PM.

Details

Summary

This is for vendors and environments that don't want to allow their users to report bugs directly to us.
By setting InteractionAllowed to false in drkonqirc the following will change:

  • For automatically restarted services, no hint is shown whatsoever
  • For application crashes a notification is shown with a Restart button but no status icon nor "Report bug" button

The DrKonqi dialog itself is unchanged (for now?), which will show up in the rare case no notification service is running.
Also removed the icon name stuff that clearly never worked.

Test Plan

Put the following in my ~/.config/drkonqirc

[General]
InteractionAllowed=false
  • Ran killall -6 plasmashell, plasmashell restarted without a trace
  • Ran killall -6 dolphin, Dolphin crashed, and I got a new notification. Successfully managed to restart Dolphin.


Old behavior with this setting to true (default) is unchanged

  • Verified that there are no stale drkonqi instances under any circumstances

Diff Detail

Repository
R871 DrKonqi
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
broulik created this revision.Aug 19 2019, 1:25 PM
Restricted Application added a project: Plasma. · View Herald TranscriptAug 19 2019, 1:25 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
broulik requested review of this revision.Aug 19 2019, 1:25 PM
broulik edited the summary of this revision. (Show Details)

Makes sense to me.
There are situations with no useful backtraces and users can't install them, but it's nice to have some feedback than just a window completely disappearing.

++

sitter added a subscriber: sitter.Aug 19 2019, 4:32 PM

Whatever happened to m_iconName?

Code looks sound.

src/main.cpp
225

Surely it should also return right away if isShuttingDown down and interaction isn't allowed, otherwise you create a dump needlessly on account of the user not being meant to report it.

src/statusnotifier.cpp
144

Please make at least the new string i18nc informing translators that it's a notification message and should be kept brief.

Whatever happened to m_iconName?

It did a KService::serviceByStorageId(crashedApp->fakeExecutableBaseName()); lookup, e.g. for "dolphin" and doesn't find "org.kde.dolphin". So this stopped working at some point without anyone caring.

src/statusnotifier.cpp
144

I don't think that is neccessary,
The keep short in the buttons was mostly because back then notification action buttons were placed on the right side of notifications and were severly limited in size.

sitter added inline comments.Aug 20 2019, 9:03 AM
src/statusnotifier.cpp
144

I care more about the context that this is a notification message than about the brevity recommendation.

broulik added inline comments.Mon, Aug 26, 8:59 AM
src/statusnotifier.cpp
144

We never do that. Do we have a @context for that or should I just write i18nc("Notification text", "...")?

sitter added inline comments.Mon, Aug 26, 9:08 AM
src/statusnotifier.cpp
144

We do not. I always check my handy cheat sheet http://people.ubuntu.com/~apachelogger/misc/i18nccheatsheet.png ;)

i18nc("Notification text", "...") lgtm

broulik updated this revision to Diff 64647.Mon, Aug 26, 9:16 AM
  • Add i18nc
Restricted Application added a project: Dolphin. · View Herald TranscriptMon, Aug 26, 9:16 AM
Restricted Application added a subscriber: kfm-devel. · View Herald Transcript
sitter added inline comments.Mon, Aug 26, 9:21 AM
src/main.cpp
225

What about this?

broulik updated this revision to Diff 64649.Mon, Aug 26, 9:24 AM
  • Do nothing when shutting down in non-interactive mode
sitter accepted this revision.Mon, Aug 26, 9:26 AM

Thanks

This revision is now accepted and ready to land.Mon, Aug 26, 9:26 AM
This revision was automatically updated to reflect the committed changes.
This comment was removed by bcooksley.
This comment was removed by bcooksley.
bcooksley removed a subscriber: fsitter.