we should update the bubble to not show users the options (allow all plugin) and instead indicate that tiny content was blocked, and only give an option to run all flash content this time (session).

We should also make the "run all content" work even when enterprise policy is set to ASK (currently it's not clickable)

there are 2 situations where Flash is blocked that use the current omibox window to notify the user when flash is blocked :
1. All Flash is blocked (due to policy or user preference - global or for that domain)
2. Flash is allowed for the site, but PPS Tiny engages and blocks some flash content.

In both cases, the user sees "Plugin blocked' in right omnibox, and if user clicks on puzzle piece, they see a window that is not entirely helpful. This bug proposes to change that window to:
a. remove 2 radio buttons & link to "manage plugins" - neither have a place now.
b. update text in window to be more descriptive of state
c. change "learn more" link to point to more accurate help topic (to be created)

see attached for proposal :

+--------------------------------------------------+
| <strExplanation>                 _learn more_    |
|                                                  |
| _run all flash content this time_                |
|                                     [Done]       |
+--------------------------------------------------+

for above cases :
1. all flash blocked :
-<strExplanation> : "All Flash content is blocked on this page"
-_run_all_ link : disabled

2. PPSTiny blocked :
-<strExplanation> : "Some Flash content was blocked on this page:"
-_run_all_ link : enabled

Thoughts?

Deleted: newFlashDialog.jpg 12.1 KB

	newFlashDialog.jpg 12.1 KB View Download

Sounds good to me :). I wasn't aware that we display the page action if completely blocked on the page. I'm assuming that only happens if there is no fallback content for a plugin (such that a plugin placeholder gets shown). What's the motivation for that?

 Issue 666534  has been merged into this issue.

I merged the other issue in. Let's make this happen with these two requirements:

1. Works even if enterprise policy is on ASK.
2. Popup visible (somehow) even if Omnibox hidden (like in Window mode). See previously merged in bug for more details on that.

On third thought, let's make this happen and merge to M56, otherwise, I think users may be in for some pain.

I'll take this on.

Hey raymes:

Can you reconfirm when you said "Run all content" is not clickable when Enterprise policy is set? Testing on Tip of Tree, both these URLs seem to have a clickable Run All Flash (even though the radio boxes are disabled): 

http://pps-test-a.appspot.com/small_only.html
http://pps-test-a.appspot.com/tiny4.html

The relevant line of code seems to be: https://cs.chromium.org/chromium/src/chrome/browser/ui/content_settings/content_setting_bubble_model.cc?q=content_setting_bubble_model.cc&sq=package:chromium&l=393

That yields not-managed for when the Policy is ASK (and thus the Run All link is enabled).

That's interesting - I might have been mistaken. I thought this was the case on linux but I might just be wrong.

rolfe / ericde / others: Can I have a quick UI sanity check on these changes to the Flash blocked Omnibox popup?

Summary:

 1. Remove radio buttons. They don't make sense in the HBD world. They didn't make that much sense before either.
 2. Hide "Manage..." link when setting is Policy-managed. User cannot manage anyways.
 3. Hide the "Run all plugins this time..." link when that is impossible (setting is BLOCK by Policy or BLOCK in HBD mode).

See two attached screenshots for some scenarios that might occur.

String changes and Help article URL will be in a separate patch, since we may not want to merge string changes into 56. (Correct me if wrong).

Thanks!

Deleted: Screenshot from 2016-12-07 17:01:34.png 29.3 KB

	Screenshot from 2016-12-07 17:01:34.png 29.3 KB View Download

Deleted: Screenshot from 2016-12-07 12:07:55.png 30.5 KB

	Screenshot from 2016-12-07 12:07:55.png 30.5 KB View Download

Looks good. Two small points:

1) I know we talked about this language two months ago but the ":" followed by "learn more" is getting to me. Any reason not to stick with the original language? "Plugins were blocked on this page. Learn more" (image attached) Seems you could still list one or more after that and be OK.

2) I had also recommended if you had time to remove links altogether if they don't necessarily add much a simple text-only statement is more than sufficient. "XYZ was blocked on this page. This setting is controlled by your administrator." Another option if you're still considering it.

Deleted: OriginalDialog.png 126 KB

	OriginalDialog.png 126 KB View Download

rolfe: Thanks for the review. Sounds like the limited patch I have screenshots of in c#11 is good.

I'm totally fine with your two recommendations for the string change patch (still forthcoming). Thanks!

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

commit cee4c12b4204f5ab1211903d0b5fd0da8b8b5c90
Author: tommycli <tommycli@chromium.org>
Date: Fri Dec 09 21:08:51 2016

[HBD] Remove radio buttons from the Plugins Blocked Omnibox popup.

This CL does two things to the Plugins Block Omnibox popup:

 1. Removes the radio buttons. They are misleading and irrelevant in the HBD world, and weren't actually that useful in the pre-HBD world either.

 2. Hides the Run All Plugins link in the policy-managed BLOCK case or the HBD BLOCK case.

 3. Hides the Manage... link in the policy-managed case. User cannot manage the setting anyways.

BUG= 655899 

Review-Url: https://codereview.chromium.org/2552343005
Cr-Commit-Position: refs/heads/master@{#437653}

[modify] https://crrev.com/cee4c12b4204f5ab1211903d0b5fd0da8b8b5c90/chrome/browser/ui/content_settings/content_setting_bubble_model.cc
[modify] https://crrev.com/cee4c12b4204f5ab1211903d0b5fd0da8b8b5c90/chrome/browser/ui/content_settings/content_setting_bubble_model.h
[modify] https://crrev.com/cee4c12b4204f5ab1211903d0b5fd0da8b8b5c90/chrome/browser/ui/content_settings/content_setting_bubble_model_unittest.cc

Your change meets the bar and is auto-approved for M56 (branch: 2924)

ericde/raymes: Merge to 56 has conflicts (that aren't completely trivial) --

Since this is not critical, I'd recommend just not merging to 56 and letting this start in 57. Thoughts / complaints?

my concern is that we plan on making this on by default in M56. if the risk of merge includes destabilizing or impacting functional path, then I agree. I do think though that this is a really strong WANT.

ericde: Blah -- okay I can understand a strong WANT.

I'll put forth some more effort on the merge and probably ask bauerb@ to re-review the merged code.

Thx.

What would it take to make the patch land smoothly?  Could we squeak by by landing a couple of extra CLs?

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

commit 5ef06a9eb40acb489b5dc8f65872f3cb089cfc61
Author: tommycli <tommycli@chromium.org>
Date: Tue Dec 13 16:44:31 2016

[HBD] Remove radio buttons from the Plugins Blocked Omnibox popup.

This CL does two things to the Plugins Block Omnibox popup:

 1. Removes the radio buttons. They are misleading and irrelevant in the HBD world, and weren't actually that useful in the pre-HBD world either.

 2. Hides the Run All Plugins link in the policy-managed BLOCK case or the HBD BLOCK case.

 3. Hides the Manage... link in the policy-managed case. User cannot manage the setting anyways.

BUG= 655899 
NOTRY=true
NOPRESUBMIT=true

Review-Url: https://codereview.chromium.org/2552343005
Cr-Commit-Position: refs/heads/master@{#437653}
(cherry picked from commit cee4c12b4204f5ab1211903d0b5fd0da8b8b5c90)

Review-Url: https://codereview.chromium.org/2565403003
Cr-Commit-Position: refs/branch-heads/2924@{#474}
Cr-Branched-From: 3a87aecc31cd1ffe751dd72c04e5a96a1fc8108a-refs/heads/master@{#433059}

[modify] https://crrev.com/5ef06a9eb40acb489b5dc8f65872f3cb089cfc61/chrome/browser/ui/content_settings/content_setting_bubble_model.cc
[modify] https://crrev.com/5ef06a9eb40acb489b5dc8f65872f3cb089cfc61/chrome/browser/ui/content_settings/content_setting_bubble_model.h
[modify] https://crrev.com/5ef06a9eb40acb489b5dc8f65872f3cb089cfc61/chrome/browser/ui/content_settings/content_setting_bubble_model_unittest.cc

Tested the same on win10 mac 10.11.6 and Linux 14.04 chrome version 56.0.2924.28 navigating to the URL http://www.cbs.com/shows/big_bang_theory/video/8983324B-8E94-4875-C329-78505B8E2E2F/the-big-bang-theory-the-dependence-transcendence/ - Observed as below

win10 : the radio buttons are removed and Manage link is not displayed but the Run All Plugins link is displayed as shown in the screenshot

Mac : Observed radio buttons are disabled but the Run All Plugins link is displayed and a square box is displayed clicking on it redirects to chrome://settings/contentExceptions#plugins

Linux : Plugin blocked icon is not displayed

tommycli@, Could you please let me know if i am missing something here in verifying the issue.

Deleted: Mac.png 55.0 KB

	Mac.png 55.0 KB View Download

tkonchada: Can you reverify that it's 56.0.2924.28 on Mac? I would be very surprised if the radio boxes still appear.

Also try using this URL for the blocked icon: http://pps-test-a.appspot.com/tiny4.html

674062 also reports bad behavior on Mac. I'm guessing the Cocoa implementation interacts badly with our changes... Doh

 Issue 670108  has been merged into this issue.

Re: removing the plugin list and changing the top message to just say "Some Flash Content has been blocked on this page".

I think Chrome still supports loading custom Pepper plugins via command line. Those use cases (admittedly quite rare) would get a confusing Omnibox message there.

Do we still care about those people?

Tommy

The only use case that we want to support for command line loading is for the sake of Chrome/Chromium testing.  We're not making any promises for 3rd parties.

Removing myself but a good design contact for security is +maxwalker (and then bettes & hwi for desktop if needed)

This has been fixed for some time. (I think it's less confusing now. See c#23.)