Differential D23261

Add option to discourage reporting bugs
ClosedPublic
Actions

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

Details

Reviewers
sitter
Group Reviewers
Plasma
Commits
R871:81c4e8f53148: Add option to discourage reporting bugs
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
Lint Skipped
Unit
Unit Tests Skipped
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)
davidedmundson added a subscriber: davidedmundson.Aug 19 2019, 2:45 PM

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.

broulik added a comment.Aug 20 2019, 6:39 AM

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.Aug 26 2019, 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.Aug 26 2019, 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.Aug 26 2019, 9:16 AM
  • Add i18nc
Restricted Application added a project: Dolphin. · View Herald TranscriptAug 26 2019, 9:16 AM
Restricted Application added a subscriber: kfm-devel. · View Herald Transcript
sitter added inline comments.Aug 26 2019, 9:21 AM
src/main.cpp
225

What about this?

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

Thanks

This revision is now accepted and ready to land.Aug 26 2019, 9:26 AM
Closed by commit R871:81c4e8f53148: Add option to discourage reporting bugs (authored by broulik). · Explain WhyAug 26 2019, 9:59 AM
This revision was automatically updated to reflect the committed changes.
• fsitter added a subscriber: • fsitter.Aug 29 2019, 3:51 PM
This comment was removed by bcooksley.
• fsitter added a comment.Aug 29 2019, 3:51 PM
This comment was removed by bcooksley.
bcooksley added a subscriber: bcooksley.Aug 29 2019, 7:12 PM
bcooksley removed a subscriber: • fsitter.

Revision Contents
Changeset List

PathPackages
Msrc/main.cpp (22 lines)
Msrc/statusnotifier.h (7 lines)
Msrc/statusnotifier.cpp (59 lines)

Diff 64649

View Options

src/main.cpp

Loading...
View Options

src/statusnotifier.h

Loading...
View Options

src/statusnotifier.cpp

Loading...