Also, the settings option is not relevant at all for notifications that don't have the cog because there's nothing in the settings UI that allows you to modify that notification (for example, the download notification).

I'd be inclined to remove it.

+hwi for desktop UI

I'm rather reluctant to remove this. It's a very idiomatic user gesture on (especially) Windows and Action Center notifications support it with the same option. It's actually one of the things I'd like to add to MD-style notifications for Windows compat.

Hwi, what do you think?

Idiomatic user gesture is one thing but in the present case it's really there just to be there as we already have a settings affordance to disable the notification. 

I don't see the benefit in keeping this as it causes issues with long press on Chrome OS.

Then we should measure its use.

The context menu can be disabled for just Chrome OS if that's the concern.

> The context menu can be disabled for just Chrome OS if that's the concern.
That'd be fine, that's ultimately my main concern, you guys know best for Win.

What is an idiomatic gesture? Context menus? Context menus are popular on every OS, but there are also many surfaces that don't have context menus, and few context menus that have exactly one row. If it were not already redundant with other, existing UI, I'd suggest moving it to a button, but since we have the cog we should just get rid of it.

here's a screenshot from Linux. Windows is the same modulo aesthetics of menu.

Deleted: 2017-09-27.png 20.7 KB

	2017-09-27.png 20.7 KB View Download

Redundancy was an explicit goal for helping users get rid of notifications, and other platforms are moving in the same direction. Edge does the following:

https://winblogs.azureedge.net/win/2016/06/ActionCenter7.jpg

(Both a gear and long press/right click context menu.)

I'm a long time Windows user and would expect this -- thus biased. I'm happy to defer to hwi@, but fundamentally this is a UX decision.

I suggest we *keep* the context menu because:
- the context menu can also be triggered in the notification drawer, where the individual cog is not visible anymore but there's only one global cog, which is not immediately obvious.
- finding and disabling the individual site/app from the setting's list UI is not as easy as the context menu 'Disable notifications from [site origin]'.
- for mouse users, right click might be used more to disable individual notification. 

As mentioned in c5, measuring the use is needed for future improvements. 

A future consideration might be:
- Have the individual cog to open a *menu* with the 2 items,"Disable notification from [site origin]" and "Settings", instead of landing on the global settings.
- Add the individual cog on hover on notification drawer as well for consistency.
- With the 2 modifications above, remove the context menu. 

> Have the individual cog to open a *menu* with the 2 items,"Disable notification from [site origin]" and "Settings", instead of landing on the global settings.
We're planning this on Chrome OS
> Add the individual cog on hover on notification drawer as well for consistency.
Not sure I understood this one.

c12: 

Hey sgabriel@, 

> Have the individual cog to open a *menu* with the 2 items,"Disable notification from [site origin]" and "Settings", instead of landing on the global settings.
>> We're planning this on Chrome OS
Great!

> Add the individual cog on hover on notification drawer as well for consistency.
>> Not sure I understood this one.
Please disregard this. For some reason, I didn't see the cog on the notification drawer for an individual notification. I see it now.  

for c10-13, updated answer:

I suggest we *keep* the context menu because:
- finding and disabling the individual site/app from the setting's list UI is not as easy as the context menu 'Disable notifications from [site origin]'.
- for mouse users, right click might be used more to disable individual notification. 

As mentioned in c5, measuring the use is needed for future improvements. 

A future consideration might be:
- Have the individual cog to open a *menu* with the 2 items,"Disable notification from [site origin]" and "Settings", instead of landing on the global settings(Chrome OS is currently looking into this  per c12).
- With the modification above, remove the context menu. 

Additional note: for touch, it seems that removing the context menu might still be a problem due to the lack of hover event, i.e. no way to show the cog icon. We can continue thinking about this. But for now to answer this crbug, I still think we should keep the context menu. 

What is the notification drawer? The message center bubble that has a collection of notifications in it doesn't exist on Windows, AFAIK.

Things to note about the context menu:
1. It's already broken (and has been, AFAICT, since it existed). It doesn't disable the notifications in the correct profile, it always disables that notification source in the first profile. All you need is two users on desktop to demonstrate this.
2. It does not make sense or work at all on system notifications, such as downloads. It appears, but selecting "disable" effectively does nothing except to dismiss the notification one time.

Given the total lack of reliability here, it's hard to argue that users are currently relying on this menu being there. As far as I know, we haven't even had bugs filed about these issues and the only way that I noticed them is by reading the code.

(The settings pages, in contrast, do not have these issues.)

> 1. It's already broken (and has been, AFAICT, since it existed). It doesn't disable the notifications in the correct profile, it always disables that notification source in the first profile. All you need is two users on desktop to demonstrate this.

How many users actually have multiple profiles, and would thus run in to this?

> 2. It does not make sense or work at all on system notifications, such as downloads. It appears, but selecting "disable" effectively does nothing except to dismiss the notification one time.

There's a preference (prefs::kMessageCenterDisabledSystemComponentIds) in which disabled system components are stored. Whether it makes sense or not is a separate question, and can be argued either way. (I'm erring towards it not making sense.)

> Given the total lack of reliability here, it's hard to argue that users are currently relying on this menu being there. As far as I know, we haven't even had bugs filed about these issues and the only way that I noticed them is by reading the code.

This sort of statement really needs data to back it up. I suspect that the very vast majority of users won't have multiple profiles. There indeed is little anecdotal evidence -- I already suggested adding a counter in #c5.

> How many users actually have multiple profiles, and would thus run in to this?

Enough that we'd have a bug report by now if anyone used this context menu.

> There's a preference[...]

There's a preference. It's not consulted when notifications are shown, thus it doesn't work.

> This sort of statement really needs data to back it up[...]

What data would back it up? Knowing how many people use this feature isn't enough because although that number is doubtlessly vanishingly small, most features in Chrome are not used by the majority of users. And knowing how many people used this menu wouldn't tell us how many couldn't accomplish the task via other means (OIB or chrome://settings page).

+ omrilio@ for cross-platform visibility and planning.

also relevant point I haven't seen mentioned yet: I believe the context menu doesn't exist on mac

re: 19 Mac - from my understanding, the context menu discussion was for Win, Linux, and CrOS as we ship Mac-native notification only on Mac. I could be wrong. Peter - could you clarify? 

re: context menu - sgabriel@'s inline notification setting design shared via crbug.com/771269 should address the context menu question. When we implement the inline notification setting per the design, we can get rid of the context menu. The visible indicator (i.e. gear icon) is likely to be more usable than the context menu.

re: c15-17
Are issues only for the context menu? Or if we implement the inline notification setting, will there be the same issue? 

> re: context menu - sgabriel@'s inline notification setting design shared via crbug.com/771269

This bug only addresses a disabled state within the settings panel on Chrome OS. I do plan on having inline settings for all notifications (web + native) based on Android inline settings. At this point, we will remove the contextual menu on Chrome OS only, feel free to also remove it on other platform per your recommendation Hwi.

re 20: I haven't seen any designs for inline settings (the settings panel on chromeos is chromeos only, shown when you click the cog, whereas on desktop when you click the cog you go to chrome://settings). But if inline settings replace the context menu we'll still have 4 different UIs for controlling notifications (page info, chrome://settings, inline settings, cros settings panel). Having so many ways of doing the same thing means we'll always have bugs in one or another of them, forget to update one when we update the others, etc.

For c15-17: yes, these bugs apply only to the context menu. They can be fixed (I've already started on it just to unblock my other work), but I believe they demonstrate that the context menu is not used often (or we'd have noticed before I started reading the code).

There's not mock yet for inline settings in anything else than Android notification (see attached). We need to carry it to native/web for consistency and it will replace contextual.

Deleted: inline-settings.png 49.9 KB

	inline-settings.png 49.9 KB View Download

re: c22

Thanks estade@ for the clarification on the context menu issues. 

I agree on the maintenance issues. Each places seem to serve their purposes in different contexts though. In CrOS, if there's a way to show native app notification settings in chrome://settings, the CrOS settings panel might not be necessary, IMO.

with c23, we can confidently move away from context menu. Thanks sgabriel@ for the design and the plan.

> the CrOS settings panel might not be necessary, IMO.
The settings panel is actually very useful to control Android notifications because we currently hide the Android settings within Chrome OS.

c25: Helpful to know it. Thanks. On Win and Linux, the 3 other notification settings UIs are needed. We'll need to maintain all 4 UIs (4 in CrOS, and 3 in Win and Linux).

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f09e3340b2f3ca0d4973264386e69504de1997a8

commit f09e3340b2f3ca0d4973264386e69504de1997a8
Author: Evan Stade <estade@chromium.org>
Date: Mon Oct 09 19:48:07 2017

Only show context menu on notifications that have a settings button.

Thus system notifications, which can't be disabled anyway, stop showing
a context menu that is at best irrelevant and at worst broken.

TBR: sky@chromium.org,rsesek@chromium.org
Bug:  769353 , 769355 
Change-Id: Ic0310be63f4f245b0a2f2cd2ccd3bc06b116b717
Reviewed-on: https://chromium-review.googlesource.com/702641
Reviewed-by: Peter Beverloo <peter@chromium.org>
Reviewed-by: Evan Stade <estade@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Commit-Queue: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#507453}
[modify] https://crrev.com/f09e3340b2f3ca0d4973264386e69504de1997a8/ash/message_center/message_center_view.cc
[modify] https://crrev.com/f09e3340b2f3ca0d4973264386e69504de1997a8/ash/system/web_notification/web_notification_tray.cc
[modify] https://crrev.com/f09e3340b2f3ca0d4973264386e69504de1997a8/ash/system/web_notification/web_notification_tray.h
[modify] https://crrev.com/f09e3340b2f3ca0d4973264386e69504de1997a8/chrome/browser/ui/cocoa/notifications/message_center_tray_bridge.h
[modify] https://crrev.com/f09e3340b2f3ca0d4973264386e69504de1997a8/chrome/browser/ui/cocoa/notifications/message_center_tray_bridge.mm
[modify] https://crrev.com/f09e3340b2f3ca0d4973264386e69504de1997a8/chrome/browser/ui/views/message_center/web_notification_tray.cc
[modify] https://crrev.com/f09e3340b2f3ca0d4973264386e69504de1997a8/chrome/browser/ui/views/message_center/web_notification_tray.h
[modify] https://crrev.com/f09e3340b2f3ca0d4973264386e69504de1997a8/ui/message_center/fake_message_center_tray_delegate.cc
[modify] https://crrev.com/f09e3340b2f3ca0d4973264386e69504de1997a8/ui/message_center/fake_message_center_tray_delegate.h
[modify] https://crrev.com/f09e3340b2f3ca0d4973264386e69504de1997a8/ui/message_center/message_center_tray.cc
[modify] https://crrev.com/f09e3340b2f3ca0d4973264386e69504de1997a8/ui/message_center/message_center_tray_delegate.h
[modify] https://crrev.com/f09e3340b2f3ca0d4973264386e69504de1997a8/ui/message_center/message_center_tray_unittest.cc
[modify] https://crrev.com/f09e3340b2f3ca0d4973264386e69504de1997a8/ui/message_center/notification_delegate.h
[modify] https://crrev.com/f09e3340b2f3ca0d4973264386e69504de1997a8/ui/message_center/views/message_popup_collection.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0f5d5f32748df72791bb19fc96d38c0a5839b150

commit 0f5d5f32748df72791bb19fc96d38c0a5839b150
Author: Evan Stade <estade@chromium.org>
Date: Tue Oct 10 18:29:05 2017

Make notification context menu "disable notification" action work on
proper profile.

Route through NotificationDelegate, which knows about the profile,
instead of relying on NotifierId, which isn't connected to a particular
profile.

TBR: yoshiki@chromium.org
Bug:  769355 
Change-Id: I9ce6bfe4c88d636d53bb6e5b17271b5aefeef76e
Reviewed-on: https://chromium-review.googlesource.com/702720
Reviewed-by: Evan Stade <estade@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Commit-Queue: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#507726}
[modify] https://crrev.com/0f5d5f32748df72791bb19fc96d38c0a5839b150/ash/message_center/message_center_view.cc
[modify] https://crrev.com/0f5d5f32748df72791bb19fc96d38c0a5839b150/ash/message_center/message_center_view.h
[modify] https://crrev.com/0f5d5f32748df72791bb19fc96d38c0a5839b150/ash/message_center/message_center_view_unittest.cc
[modify] https://crrev.com/0f5d5f32748df72791bb19fc96d38c0a5839b150/ash/message_center/message_list_view_unittest.cc
[modify] https://crrev.com/0f5d5f32748df72791bb19fc96d38c0a5839b150/chrome/browser/notifications/web_notification_delegate.cc
[modify] https://crrev.com/0f5d5f32748df72791bb19fc96d38c0a5839b150/chrome/browser/notifications/web_notification_delegate.h
[modify] https://crrev.com/0f5d5f32748df72791bb19fc96d38c0a5839b150/ui/arc/notification/arc_notification_content_view_unittest.cc
[modify] https://crrev.com/0f5d5f32748df72791bb19fc96d38c0a5839b150/ui/arc/notification/arc_notification_view_unittest.cc
[modify] https://crrev.com/0f5d5f32748df72791bb19fc96d38c0a5839b150/ui/message_center/fake_message_center.cc
[modify] https://crrev.com/0f5d5f32748df72791bb19fc96d38c0a5839b150/ui/message_center/fake_message_center.h
[modify] https://crrev.com/0f5d5f32748df72791bb19fc96d38c0a5839b150/ui/message_center/message_center.h
[modify] https://crrev.com/0f5d5f32748df72791bb19fc96d38c0a5839b150/ui/message_center/message_center_impl.cc
[modify] https://crrev.com/0f5d5f32748df72791bb19fc96d38c0a5839b150/ui/message_center/message_center_impl.h
[modify] https://crrev.com/0f5d5f32748df72791bb19fc96d38c0a5839b150/ui/message_center/message_center_impl_unittest.cc
[modify] https://crrev.com/0f5d5f32748df72791bb19fc96d38c0a5839b150/ui/message_center/message_center_tray.cc
[modify] https://crrev.com/0f5d5f32748df72791bb19fc96d38c0a5839b150/ui/message_center/message_center_tray.h
[modify] https://crrev.com/0f5d5f32748df72791bb19fc96d38c0a5839b150/ui/message_center/message_center_tray_unittest.cc
[modify] https://crrev.com/0f5d5f32748df72791bb19fc96d38c0a5839b150/ui/message_center/notification_delegate.cc
[modify] https://crrev.com/0f5d5f32748df72791bb19fc96d38c0a5839b150/ui/message_center/notification_delegate.h
[modify] https://crrev.com/0f5d5f32748df72791bb19fc96d38c0a5839b150/ui/message_center/views/message_center_controller.h
[modify] https://crrev.com/0f5d5f32748df72791bb19fc96d38c0a5839b150/ui/message_center/views/message_popup_collection.cc
[modify] https://crrev.com/0f5d5f32748df72791bb19fc96d38c0a5839b150/ui/message_center/views/message_popup_collection.h
[modify] https://crrev.com/0f5d5f32748df72791bb19fc96d38c0a5839b150/ui/message_center/views/message_view.cc
[modify] https://crrev.com/0f5d5f32748df72791bb19fc96d38c0a5839b150/ui/message_center/views/message_view.h
[modify] https://crrev.com/0f5d5f32748df72791bb19fc96d38c0a5839b150/ui/message_center/views/message_view_context_menu_controller.cc
[modify] https://crrev.com/0f5d5f32748df72791bb19fc96d38c0a5839b150/ui/message_center/views/notification_view_md_unittest.cc
[modify] https://crrev.com/0f5d5f32748df72791bb19fc96d38c0a5839b150/ui/message_center/views/notification_view_unittest.cc
[modify] https://crrev.com/0f5d5f32748df72791bb19fc96d38c0a5839b150/ui/message_center/views/toast_contents_view.cc
[modify] https://crrev.com/0f5d5f32748df72791bb19fc96d38c0a5839b150/ui/message_center/views/toast_contents_view.h

I think I've fixed the context menu so it at least functions properly. Sounds like the long term plan is to replace with inline notification settings so for now this can be closed out. When designing inline notification settings we'll have to take into account if policy settings preclude changing the notification's setting, the source profile for the notification, and whether a given notification can have its settings tweaked (e.g. system notifications cannot).