Block Flash by default and allow enabling with CTP #116
Conversation
bbondy
changed the title
bbondy
force-pushed
the
flash
branch
2 times, most recently
from
ab1540d
to
fdd2652
Compare
bbondy
changed the title
bbondy
requested review from
yrliou and
diracdeltas
and removed request for
diracdeltas
|
Talked to @diracdeltas on Slack and she's OK with the approach. She wants the settings to expire which should happen with C68 and is tracked by the issue linked above. |
| @@ -16,6 +16,7 @@ const char kFingerprinting[] = "fingerprinting"; | |||
| const char kBraveShields[] = "braveShields"; | |||
| const char kReferrers[] = "referrers"; | |||
| const char kCookies[] = "cookies"; | |||
| const char kFlash[] = "adobe-flash-player"; | |||
There was a problem hiding this comment.
is this constant still being used?
There was a problem hiding this comment.
nope, I'll remove before landing.
| ContentSettingsPattern::FromString("http://a.com/*"); | ||
| } | ||
|
|
||
| // PermissionsBrowserTest |
There was a problem hiding this comment.
nit: remove extra spaces here to align with the next line
There was a problem hiding this comment.
Will remove the comment, it's not helpful and I think it was just copied in from somewhere.
|
|
||
| bool RunScriptReturnBool(const std::string &script) { | ||
| bool value; | ||
| EXPECT_TRUE(ExecuteScriptAndExtractBool(contents(), script, &value)); |
There was a problem hiding this comment.
nit: generally I would prefer to avoid putting EXPECT/ASSERT codes inside a function since it would be harder to see where it actually failed when you call it multiple times in a single test. I'm OK with it since currently it is called only once.
There was a problem hiding this comment.
it's only used once anyway, I'll just not have that helper.
…rewards panel Fixed getting favicon for Twitch vod Update for youtube showing up right away in panel. Stabilized youtube media publisher panel Removing debug log lines Cleaned up redundant database functionality Stabilized youtube media publisher panel clean up debug comments Replaced accidentally removed line Merge Fixes exclude/include on fresh site
Fix brave/brave-browser#30
By setting the default content setting to Block, this ensures there's at least a couple steps needed to get Flash working.
If Flash is not installed:
navigator.pluginsobviously doesn't show Flash here.No extra code nor UI for that.
If Flash is installed:
navigator.pluginsdoesn't show Flash here.Clicking on the icon in the URL bar:
navigator.pluginsdoesn't show Flash here.Page reloads with Click to Play:
navigator.pluginsdoesn't show Flash here.Clicking on the Flash content:
navigator.pluginsdoesn't show Flash here.Finally Flash is played:
navigator.pluginswill show Flash here.Limitations:
Submitter Checklist:
git rebase -ito squash commits (if needed).Test Plan:
Reviewer Checklist: