cros: message center notification settings: screenshot? |
||||||||
Issue descriptionIn the message center settings bubble, you can enable or disable notifications from different sources, like web pages, arc applications, extensions, and, for some reason, the built-in chromeos screenshot notification. Why is the screenshot notification disable-able, while other system notifications, e.g. for downloads, are not? Can we remove this exception to the standard rules? as a note to myself and other engineers, here[1] is where system component sources are added to the list of notifiers, and as you can see only the screenshot one is added. In comparison, here[2] is where individual entries are added for each extension that's installed and has the notification permission. [1] https://cs.chromium.org/chromium/src/chrome/browser/notifications/system_component_notifier_source_chromeos.cc?rcl=9278b89f274a38f82683ffe3eb51e7ffc56693fa&l=21 [2] https://cs.chromium.org/chromium/src/chrome/browser/notifications/application_notifier_source.cc?rcl=6f91dde5f828c3a5e1c9e01eecd8f2e2b4789f6f&l=35
,
Sep 21 2017
That's fair. I' be ok removing it. +Jenn for UX
,
Sep 25 2017
Is this OK to remove or should we wait for Jenn to weigh in?
,
Sep 25 2017
Sorry to be slow here — before removing (which I'd be supportive of), I'd like to just make sure we have a sense of number of users this would affect. Do we have stats on how many folks have disabled this?
,
Sep 25 2017
I don't see any UMA that logs notifications being disabled. +peter
,
Sep 27 2017
+stevenjb in case he knows of relevant UMA. In lieu of UMA stats, what is the action here?
,
Sep 27 2017
I'm not familiar with that specific notification, and it's been a long time since I've been in that code; I don't recall what UMA stats may or may not exist.
,
Sep 28 2017
Happy to let it go then; thanks for doing due diligence!
,
Sep 28 2017
woohoo, thanks.
,
Oct 2 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8b46b61255b476c7d8c75026227daf39d223ff35 commit 8b46b61255b476c7d8c75026227daf39d223ff35 Author: Evan Stade <estade@chromium.org> Date: Mon Oct 02 21:23:14 2017 Remove ash system screenshot from blockable notification list. Bug: 766846 , 769353 Change-Id: I48a8b72f787c4e96f517ee2ed60be32bc977f550 Reviewed-on: https://chromium-review.googlesource.com/689743 Commit-Queue: Evan Stade <estade@chromium.org> Reviewed-by: Steven Bennetts <stevenjb@chromium.org> Cr-Commit-Position: refs/heads/master@{#505788} [modify] https://crrev.com/8b46b61255b476c7d8c75026227daf39d223ff35/chrome/browser/BUILD.gn [modify] https://crrev.com/8b46b61255b476c7d8c75026227daf39d223ff35/chrome/browser/notifications/message_center_settings_controller.cc [modify] https://crrev.com/8b46b61255b476c7d8c75026227daf39d223ff35/chrome/browser/notifications/message_center_settings_controller_unittest.cc [modify] https://crrev.com/8b46b61255b476c7d8c75026227daf39d223ff35/chrome/browser/notifications/notifier_state_tracker.cc [modify] https://crrev.com/8b46b61255b476c7d8c75026227daf39d223ff35/chrome/browser/notifications/notifier_state_tracker.h [delete] https://crrev.com/a77e434c2c222ede9780ca292510692f601acca5/chrome/browser/notifications/system_component_notifier_controller_chromeos.cc [delete] https://crrev.com/a77e434c2c222ede9780ca292510692f601acca5/chrome/browser/notifications/system_component_notifier_controller_chromeos.h [modify] https://crrev.com/8b46b61255b476c7d8c75026227daf39d223ff35/chrome/browser/ui/ash/chrome_screenshot_grabber.cc
,
Oct 2 2017
,
Jan 22 2018
,
Jan 23 2018
|
||||||||
►
Sign in to add a comment |
||||||||
Comment 1 by weifangsun@chromium.org
, Sep 21 2017