Closed Bug 1298083 Opened 8 years ago Closed 8 years ago

Don't hide blocklisted Flash from navigator.plugins

Categories

(Core Graveyard :: Plug-ins, defect, P2)

defect

Tracking

(firefox52 fixed)

RESOLVED FIXED
mozilla52
Tracking Status
firefox52 --- fixed

People

(Reporter: benjamin, Assigned: blassey)

References

Details

Attachments

(1 file)

When Firefox is set up to hide click-to-play Flash from navigator.plugins, this shouldn't affect the case where Flash is click-to-play because it is marked as vulnerable.

The reason for doing this is risk mitigation: the current UI has a risk of false-negatives which means sites become unusable, and that may affect our ability to deploy blocks. For now, we don't want any of this work to affect plugin blocking.
Blocks: 1186948
Priority: -- → P2
been working on a test for this for a while and coming up empty.
Attachment #8789057 - Flags: review?(mconley)
Comment on attachment 8789057 [details] [diff] [review]
dont_hide_blocklisted.patch

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

Sorry it's taken me so long to get to this - eating my way up through my review queue now.

This looks reasonable, but I don't really think I'm comfortable reviewing stuff in nsPluginArray.cpp. My experience with the more platform-y parts of plugins is limited at best. You might want bsmedberg to give this the go ahead.
Attachment #8789057 - Flags: review?(mconley)
Attachment #8789057 - Flags: review?(benjamin)
Attachment #8789057 - Flags: feedback+
Comment on attachment 8789057 [details] [diff] [review]
dont_hide_blocklisted.patch

I'm not sure this is correct, but I had trouble following the code.

For the case we care about here, the blocklist state should be STATE_VULNERABLE_UPDATE_AVAILABLE or STATE_VULNERABLE_NO_UPDATE. That's the case where we end up in a click-to-activate setting by default. The case where the plugin is hardblocked doesn't really apply to Flash.

I think translates to a plugin tag that is *not* IsBlocklisted(). The best check there is probably GetBlockListState() != 0.

r=me with that change
Attachment #8789057 - Flags: review?(benjamin) → review+
Without an automated test, we should definitely get some manual verification from Michelle.
QA Contact: mfunches
QA Update: yes I can arrange for testing
Pushed by blassey@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/471872efc401
Don't hide blocklisted Flash from navigator.plugins r=bsmedberg
https://hg.mozilla.org/mozilla-central/rev/471872efc401
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
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: