Closed
Bug 795397
Opened 12 years ago
Closed 12 years ago
click-to-play blocklisting: respect the "plugins" permission
Categories
(Core Graveyard :: Plug-ins, defect)
Tracking
(firefox17 fixed, firefox18 verified)
RESOLVED
FIXED
mozilla18
People
(Reporter: keeler, Assigned: keeler)
References
Details
Attachments
(1 file, 2 obsolete files)
7.20 KB,
patch
|
keeler
:
review+
akeybl
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
Initially I thought it would be a good idea for a click-to-play-blocklisted plugin to always be click-to-play, regardless of the "plugins" permission value. (The reasoning being, if a user has in the past said "always allow plugins" on a site, and then we make a plugin click-to-play, they should be made aware of the security implications of running that plugin.) However, since non-blocklist click-to-play isn't a first-class feature yet anyway, this a) won't happen to many users and b) isn't the right behavior anyway for the rest of users. Ideally, we would have a more sophisticated plugins-permission manager that would handle all of these edge cases and actually differentiate between types of plugins and whether or not a plugin was vulnerable when it was allowed to run, but we're a ways away from that. In the meantime, it would be best if the permissions simply worked as expected.
Assignee | ||
Comment 1•12 years ago
|
||
Josh - if you could review the nsObjectLoadingContent changes, that'd be great.
Assignee | ||
Comment 2•12 years ago
|
||
Comment on attachment 666028 [details] [diff] [review] patch And Jared, if you could review the test changes, that would also be great.
Attachment #666028 -
Flags: review?(jaws)
Comment 3•12 years ago
|
||
Comment on attachment 666028 [details] [diff] [review] patch Review of attachment 666028 [details] [diff] [review]: ----------------------------------------------------------------- r+ for the test changes.
Attachment #666028 -
Flags: review?(jaws) → review+
Comment 4•12 years ago
|
||
Comment on attachment 666028 [details] [diff] [review] patch Review of attachment 666028 [details] [diff] [review]: ----------------------------------------------------------------- ::: content/base/src/nsObjectLoadingContent.cpp @@ +2525,5 @@ > + // set the fallback reason > + // (if it's click-to-play, it might be because of the blocklist) > + uint32_t state; > + nsresult rv = pluginHost->GetBlocklistStateForType(mContentType.get(), &state); > + NS_ENSURE_SUCCESS(rv, false); nit, if this call fails for some reason, we might return false here without setting aReason (an out param), which would result in callers using uninitialized values for fallback reasons.
Comment on attachment 666028 [details] [diff] [review] patch Review of attachment 666028 [details] [diff] [review]: ----------------------------------------------------------------- r+ with a fix for the nit :johns pointed out
Attachment #666028 -
Flags: review?(joshmoz) → review+
Assignee | ||
Comment 6•12 years ago
|
||
Fixed it so the aReason gets set. Carrying over r+. Try run: https://tbpl.mozilla.org/?tree=Try&rev=0281ae49e05e (only running on one platform to conserve try resources and because there are no platform-specific changes. If this blows up in my face I'll go back to running on everything :)
Attachment #666028 -
Attachment is obsolete: true
Attachment #666608 -
Flags: review+
Assignee | ||
Updated•12 years ago
|
Keywords: checkin-needed
Comment 7•12 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/68bf9d057cf4 A real commit message would have been nice :-(
Flags: in-testsuite+
Keywords: checkin-needed
Comment 8•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/68bf9d057cf4
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
Assignee | ||
Comment 9•12 years ago
|
||
(In reply to Ryan VanderMeulen from comment #7) > https://hg.mozilla.org/integration/mozilla-inbound/rev/68bf9d057cf4 > > A real commit message would have been nice :-( Sorry about that :-/
Target Milestone: mozilla18 → ---
Assignee | ||
Comment 10•12 years ago
|
||
... and the target milestone got reset. I'm doing really well lately...
Target Milestone: --- → mozilla18
Assignee | ||
Comment 11•12 years ago
|
||
[Approval Request Comment] Bug caused by (feature/regressing bug #): blocklist/click-to-play plugins (bug 760625) User impact if declined: the "always activate plugins" option in the popup notification won't work for blocklisted plugins Testing completed (on m-c, etc.): tested on m-c Risk to taking this patch (and alternatives if risky): low (potentially could regress click-to-play plugins, but we've seen no evidence of that yet) String or UUID changes made by this patch: none
Attachment #667549 -
Flags: review+
Attachment #667549 -
Flags: approval-mozilla-aurora?
Assignee | ||
Updated•12 years ago
|
Attachment #666608 -
Attachment is obsolete: true
Updated•12 years ago
|
Attachment #667549 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Assignee | ||
Updated•12 years ago
|
Keywords: checkin-needed
Whiteboard: [patch needs uplift to Aurora]
Comment 12•12 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/29e52e3219a3
status-firefox17:
--- → fixed
status-firefox18:
--- → fixed
Keywords: checkin-needed
Whiteboard: [patch needs uplift to Aurora]
Assignee | ||
Comment 14•11 years ago
|
||
(In reply to Paul Silaghi [QA] from comment #13) > Is there a way to undo these settings? At the moment, Tools -> Clear Recent History -> Site Preferences :-/ I'm working on more accessible ui in bug 775857 and bug 821892.
Comment 15•11 years ago
|
||
(In reply to Paul Silaghi [QA] from comment #13) > Is there a way to undo these settings? You can also find the site in question in about:permissions and select "forget this site"
Comment 16•11 years ago
|
||
The plugin permissions are remembered. Verified fixed on Mozilla/5.0 (Windows NT 6.1; WOW64; rv:18.0) Gecko/20100101 Firefox/18.0b4 But I personally think this shouldn't land until bug 775857 and bug 821892 are fixed. Our users won't know to undo their settings. Clicking "Never activate plugins" by mistake on youtube or facebook will have a big negative impact.
Assignee | ||
Comment 17•11 years ago
|
||
Actually, I looked into this a bit more. Bug 746374 made the change that caused "Page Info" and about:preferences to not work for this. That landed in 20, so until then, these two methods work just fine for undoing "always-" or "never activate for this site".
Assignee | ||
Comment 18•11 years ago
|
||
(In reply to David Keeler from comment #17) > about:preferences by which I mean about:permissions, of course
Comment 19•11 years ago
|
||
Please see https://bugzilla.mozilla.org/show_bug.cgi?id=775857#c4
Updated•2 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•