Closed Bug 819555 Opened 12 years ago Closed 11 years ago

click-to-play: allow for session when using popup notification

Categories

(Core Graveyard :: Plug-ins, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED WORKSFORME

People

(Reporter: keeler, Assigned: keeler)

References

Details

Attachments

(1 file)

I was originally intending this to be part of bug 810082, but I'm splitting it off so we're not doing too much in one bug.

The problem, as I see it: scripts can fail and end up in broken states when failing to handle the case when plugins are click-to-play. Unfortunately, activating plugins after the fact doesn't help - these pages are still inoperable. I think a natural user reaction at that point is to refresh the page. So, I'm proposing that if a user activates plugins via the popup-notification, this adds a temporary (session) permission for plugins on that site. This also helps with the youtube playlist example, where the only other option would be to whitelist youtube permanently.

Now, we could maybe do some fancy detection where we allow plugins if we saw that the user activated one and soon thereafter refreshed the page. However, this would be difficult to get right and it complicates the question of when plugins should get activated. I think using session permissions keeps this question simple and straightforward.

Additionally, we could add session permission when a user activates a plugin just by clicking on it. Again, though, I don't think this is a good idea, because clicking one plugin is a more lightweight activity than opening the popup notification and activating all of them, and we should similarly limit the consequences of that action.
Attached patch patchSplinter Review
Here's how it would look in patch form.
Attachment #689929 - Flags: review?(jaws)
Attachment #689929 - Flags: feedback?(jschoenick)
Comment on attachment 689929 [details] [diff] [review]
patch

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

This looks fine patch-wise, but it conflicts with the idea of bug 742753. If we only activate the single element the user clicks on, it hardly makes sense to activate all plugins on that site on further navigation.

Perhaps we could have two "modes" for click-to-play - one with per-element granularity, and one that activates all plugins on that page and sets a session permission. CTP blocklisting could default to the latter, and plugins.click_to_play to the former. When the user bypasses the blocklist, they arguably don't want to have to deal with it for every plugin element -- but when I have CTP explicitly enabled, I would not want it to blanket-enable itself for sites (or at least have a preference to flip that it does not)
Attachment #689929 - Flags: feedback?(jschoenick) → feedback+
Err, after reading more carefully I realized this is only targeted at the 'activate all addons' function, which makes more sense. Disregard previous comment.
Is that permission surviving session-restore? That might be undesirable.
How long is the term of "session"? Does it continue until Firefox will be closed, or leave the current visiting page?

If "session" means "until Firefox will be closed", I don't agree this change. 

Nowadays, a browser live in process long time. Even if user trust the safety page at the starting of browser process, after some days, its page might become untrusted site by cracking at the ending of browser process. In this case, click-to-play cannot block plugin attacking.
Of course, Firefox has the feature to block plugins automatically which have vulnerability or zero-day risk. However, there is a lag between starting zero-day attack and providing blacklist to block plugins.
In such case, if the term of "session" which we discuss is long one, click-to-play cannot guard user from security zero-day risk in the lag time.
Comment on attachment 689929 [details] [diff] [review]
patch

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

I would still like to keep a way for one-off activation of plugins on a page.

Can you explain the use case of a user who doesn't want to always activate but wants to activate for this session? I don't see why a user would trust a site like YouTube for their session (which for some users lasts over 30 days) but can't accept the "Always Activate" option.

::: browser/base/content/browser-plugins.js
@@ +586,2 @@
>      let pluginHost = Cc["@mozilla.org/plugin/host;1"].getService(Ci.nsIPluginHost);
> +    if (typeof(aExpireType) === "undefined") {

nit: no need for parens on typeof since it is a unary operator, not a function call.
if (typeof aExpireType == "undefined") {

::: browser/base/content/test/browser_pluginnotification.js
@@ +1140,5 @@
> +  gNextTest = test26b;
> +
> +  // "click" "Activate All Plugins"
> +  notification.mainAction.callback();
> +  waitForCondition(condition, function() { gTestBrowser.reload(); }, "Test 26a, Waited too long for plugin to activate");

nit: move the declaration of |condition| to be above the waitForCondition call, here and below.
Attachment #689929 - Flags: feedback?
Attachment #689929 - Flags: review?(jaws)
Attachment #689929 - Flags: feedback?
A possible alternative for a general fix for those scenarios would be some "reload this page with plugins activated" button in the c2p location bar icon menu.
> I don't see why a user would trust a site like YouTube 
> for their session but can't accept the "Always Activate" option.

There are a lot of pages where one wants plugins disabled. For example
in blocks. Everybody can put content there. But sometimes you want to watch that video.
Fixed in the new UI.
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → WORKSFORME
I wanted to write "in blogs" :-)
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: