New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 666774 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Dec 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Mac
Pri: 1
Type: Bug



Sign in to add a comment

Inconsistent Flash behavior when DefaultPluginsSetting is enabled

Project Member Reported by georgesak@chromium.org, Nov 18 2016

Issue description

I'm doing test with the following two pages:
- Page 1: pps-test-a.appspot.com/small_only.html
- Page 2: pps-test-b.appspot.com/iframe-to-a.html

And I'm seeing inconsistent behavior between both when DefaultPluginsSetting is enabled.

For example, when set to Block (value of 2), we get:
- Page 1 shows text with "Flash not supported" and "Download flash" link. Clicking the link gives the plugins blocked in the omnibox.
- Page 2 shows the puzzle piece (click to enable Adobe Flash Player). Clicking the link gives the plugins blocked in the omnibox.

So the behavior is ultimately correct (flash is blocked), however the presentation is inconsistent. It also doesn't make sense to prompt the user to enable Flash if it's blocked.

If the policy is set to Click to Play (value 3), the same presentation is offered. However this time, when the user clicks either the download link of the puzzle piece, a prompt is shown to allow flash. This behavior is again correct, however it's awkward for a user to have to click on "Download flash" to get the prompt.

This was tested in the latest Canary. Let me know if you need more information.
 
Hey -

So for DefaultPluginsSetting == 2

Page 1: pps-test-a.appspot.com/small_only.html is reasonable.
Page 2: pps-test-b.appspot.com/iframe-to-a.html has misleading text?

And for DefaultPluginsSetting == 3,

Both are okay?

Tommy
Correct for DefaultPluginsSetting == 2

For DefaultPluginsSetting == 3
Page 1 has misleading text (asks user to download flash).
Page 2 is correct.
To reconfirm:

You mean correct for == 3 (ASK) right?

And incorrect for == 2 (BLOCK) right?
Cc: ericde@chromium.org groby@chromium.org lafo...@chromium.org
ericde/groby/laforge:

We need a new message for BLOCKed plugins under HBD. I recommend "Flash is blocked"

In the pre-HBD world, BLOCKed plugins (not blocked by policy) were: "Right-click to run Flash". and BLOCK_BY_POLICY plugins were "Flash is blocked by enterprise policy".

Now with HBD, BLOCK cannot be right-clicked or clicked to play. 

I recommend simply "Flash is blocked". I just wanted one of you three to sanity check this.
Project Member

Comment 5 by bugdroid1@chromium.org, Nov 30 2016

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

commit e97656b8c9bb4fb2456673f4cd3d8eeb93a65bb8
Author: tommycli <tommycli@chromium.org>
Date: Wed Nov 30 16:43:00 2016

[HBD] Fix the misleading no-fallback placeholder message in BLOCK mode.

Previously, if the user had the BLOCK Content Setting (either from
Policy or just from user setting), the no-fallback placeholder for
HTML5 By Default would still say "Click to enable Flash".

This was misleading because that would not work. This updates the BLOCK
no-fallback placeholder to say "Flash is blocked" and "Flash is blocked
by enterprise policy" (if blocked by policy).

BUG= 666774 

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

[modify] https://crrev.com/e97656b8c9bb4fb2456673f4cd3d8eeb93a65bb8/chrome/app/generated_resources.grd
[modify] https://crrev.com/e97656b8c9bb4fb2456673f4cd3d8eeb93a65bb8/chrome/browser/plugins/plugin_info_message_filter.cc
[modify] https://crrev.com/e97656b8c9bb4fb2456673f4cd3d8eeb93a65bb8/chrome/browser/plugins/plugin_info_message_filter.h
[modify] https://crrev.com/e97656b8c9bb4fb2456673f4cd3d8eeb93a65bb8/chrome/browser/plugins/plugin_info_message_filter_unittest.cc
[modify] https://crrev.com/e97656b8c9bb4fb2456673f4cd3d8eeb93a65bb8/chrome/common/render_messages.h
[modify] https://crrev.com/e97656b8c9bb4fb2456673f4cd3d8eeb93a65bb8/chrome/renderer/chrome_content_renderer_client.cc

Labels: Merge-Request-56 Merge-Request-55

Comment 7 by gov...@chromium.org, Nov 30 2016

Cc: anan...@chromium.org mmoss@chromium.org
Labels: -Merge-Request-55 Merge-Rejected-55
We're VERY close to M55 stable promotion. CL listed at #5 indicates string change and an unlocalized string doesn't belong in Stable. So rejecting merge for M55.
govind: Alright, that's fine. Sounds like it's not worth the merge to 55. Thanks for the consideration.

Comment 9 by dimu@chromium.org, Dec 1 2016

Labels: -Merge-Request-56 Merge-Approved-56 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M56 (branch: 2924)
Project Member

Comment 10 by bugdroid1@chromium.org, Dec 1 2016

Labels: -merge-approved-56 merge-merged-2924
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ab01d18583ecfd78d9084d66434d77072d7afde1

commit ab01d18583ecfd78d9084d66434d77072d7afde1
Author: Tommy C. Li <tommycli@chromium.org>
Date: Thu Dec 01 18:24:32 2016

[HBD] Fix the misleading no-fallback placeholder message in BLOCK mode.

Previously, if the user had the BLOCK Content Setting (either from
Policy or just from user setting), the no-fallback placeholder for
HTML5 By Default would still say "Click to enable Flash".

This was misleading because that would not work. This updates the BLOCK
no-fallback placeholder to say "Flash is blocked" and "Flash is blocked
by enterprise policy" (if blocked by policy).

BUG= 666774 

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

Review URL: https://codereview.chromium.org/2549503003 .

Cr-Commit-Position: refs/branch-heads/2924@{#253}
Cr-Branched-From: 3a87aecc31cd1ffe751dd72c04e5a96a1fc8108a-refs/heads/master@{#433059}

[modify] https://crrev.com/ab01d18583ecfd78d9084d66434d77072d7afde1/chrome/app/generated_resources.grd
[modify] https://crrev.com/ab01d18583ecfd78d9084d66434d77072d7afde1/chrome/browser/plugins/plugin_info_message_filter.cc
[modify] https://crrev.com/ab01d18583ecfd78d9084d66434d77072d7afde1/chrome/browser/plugins/plugin_info_message_filter.h
[modify] https://crrev.com/ab01d18583ecfd78d9084d66434d77072d7afde1/chrome/browser/plugins/plugin_info_message_filter_unittest.cc
[modify] https://crrev.com/ab01d18583ecfd78d9084d66434d77072d7afde1/chrome/common/render_messages.h
[modify] https://crrev.com/ab01d18583ecfd78d9084d66434d77072d7afde1/chrome/renderer/chrome_content_renderer_client.cc

Status: Fixed (was: Assigned)
Labels: Needs-Feedback
Tested the issue on Mac OS 10.11.6 , Windows 10 and Ubuntu 14.04 using chrome dev M56 #56.0.2924.14 with the url's mentioned in comment #1. When Clicked on "control-click to runadobe flash player" runs the flash player and video is played.

Attached screencast for reference.

@tommycli-- Could you please confirm , is this the expected behavior??

Thanks !
666774.mp4
374 KB View Download

Sign in to add a comment