Paul Silaghi, QA [:pauly] Reporter
	Description • 12 years ago

STR:
1. Make sure plugins.click_to_play pref is set on TRUE in about:config
2. Open http://www.youtube.com/
3. Click on a video
4. Click on the plugin icon in the location bar
5. Choose to "Never activate plugins for this site"
6. Click on the video screen

Expected results:
The video should not play

Actual results:
The video starts playing

	Dana Keeler (she/her) (use needinfo) (:keeler for reviews) Assignee
	Comment 1 • 12 years ago

To clarify, are you saying that if a user chooses "Never activate plugins...", when they then click on a plugin, it should not play? If so, I think this might confuse users ("I clicked on the plugin, but it's not playing?"). In fact, I think we should get rid of the "never activate plugins..." option altogether, because all we're really saying there is "never activate plugins automatically on this site", which is what happens if you have click-to-play enabled in the first place.

	Jared Wein [:jaws] (please needinfo? me)
	Comment 2 • 12 years ago

Fixing this bug is pretty simple, we should just iterate through the plugins array and hide the overlay.

As far as the ability to always deny, the benefit is that viewing pages doesn't show the overlay and the notification icon won't appear. I don't think we should remove this ability.

	Jared Wein [:jaws] (please needinfo? me)
	Comment 4 • 12 years ago

Comment on attachment 622134 [details] [diff] [review]
patch + test

Review of attachment 622134 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good! Thanks :)

	Jared Wein [:jaws] (please needinfo? me)
	Comment 5 • 12 years ago

Comment on attachment 622134 [details] [diff] [review]
patch + test

Review of attachment 622134 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with the one change mentioned below.

::: browser/base/content/browser.js
@@ +7333,5 @@
>                              messageString, "plugins-notification-icon",
>                              mainAction, secondaryActions, options);
>    },
>  
> +  removeClickToPlayOverlays: function (aContentWindow) {

Please put an underscore in front of the function name here and name the anonymous function PH_removeClickToPlayOverlays.

	Dana Keeler (she/her) (use needinfo) (:keeler for reviews) Assignee
	Comment 6 • 12 years ago

Updated patch per jaws' comments. Carrying over r+
Try run: https://tbpl.mozilla.org/?tree=Try&rev=9c46687a0659

	Ryan VanderMeulen [:RyanVM]
	Comment 7 • 12 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/23f45cac342f

	Ed Morley [:emorley]
	Comment 8 • 12 years ago

https://hg.mozilla.org/mozilla-central/rev/23f45cac342f

	Paul Silaghi, QA [:pauly] Reporter
	Comment 9 • 12 years ago

Black screen instantly appears when choosing to "Never activate plugins for this site".
Verified fixed on FF 15.0a1 (2012-05-10) on Win 7/64, Ubuntu 12.04 and Mac OS X 10.7.

	Paul Silaghi, QA [:pauly] Reporter
	Comment 10 • 12 years ago

(In reply to Jared Wein [:jaws] from comment #2)
> As far as the ability to always deny, the benefit is that viewing pages
> doesn't show the overlay and the notification icon won't appear. I don't
> think we should remove this ability.

One disadvantage of this would be that if someone else will use the computer or event the owner after a long time might don't know why youtube.com apparently doesn't work.
In fact it's the same functionality, both with or without the CtP notification.