Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Allow flash loads flash content even when flash is not enabled in settings #6739

Closed
srirambv opened this issue Jan 19, 2017 · 10 comments
Closed

Allow flash loads flash content even when flash is not enabled in settings #6739

srirambv opened this issue Jan 19, 2017 · 10 comments
Assignees
@diracdeltas
Labels
plugin/flash QA/checked-Win32 QA/checked-Win64 QA/test-plan-specified regression release-notes/exclude security
Milestone
0.13.0

Comments

@srirambv
Copy link
Collaborator

srirambv commented Jan 19, 2017 •
edited by luixxiul
Loading

Did you search for similar issues before submitting this one?

Describe the issue you encountered:
Allow flash loads flash content even when flash is not enabled in settings

Expected behavior:
Flash should be enabled in settings to allow flash content to run

  • Platform (Win7, 8, 10? macOS? Linux distro?):
    Windows 10 x64

  • Brave Version (revision SHA):
    rev c60b783
    Muon 2.0.12

  • Steps to reproduce:

    1. Disable flash in settings page under security
    2. Visit http://www.y8.com/games/superfighters
    3. Right click on the flash placeholder and select Allow/Allow for 1 week
    4. Flash content is loaded even when flash is disabled in settings

  • Screenshot if needed:
    flash

  • Any related issues:

Test plan

See: aab8142
@srirambv srirambv added this to the 0.13.1 milestone Jan 19, 2017
@diracdeltas
Copy link
Member

just to clarify, the expected behavior that brianj, brendan, and i agreed on was:

  1. flash not installed, not enabled: no placeholders (therefore no way to run flash)
  2. flash installed, not enabled: same as above
  3. flash not installed, enabled: impossible because we don't offer the option to enable Flash until you install it
  4. flash installed, enabled: placeholders that you can right-click to run flash

in all cases above, sites that are not Adobe (or other exceptions) cannot see whether flash is installed until you click-to-play on the placeholder.

@diracdeltas diracdeltas modified the milestones: 0.13.0, 0.13.1 Jan 19, 2017
@bbondy
Copy link
Member

bbondy commented Jan 19, 2017

This is a bug and should work as you described but would you be against treating it as a bug and fixing within the context of 0.13.1? I'd rather not block on this so we can release tomorrow and the flash plugin won't actually run until you explicitly allow.

@diracdeltas diracdeltas self-assigned this Jan 20, 2017
@diracdeltas
Copy link
Member

as a hotfix i can just change the context menu to say 'enable flash in preferences' instead of 'allow flash'

@bbondy
Copy link
Member

bbondy commented Jan 20, 2017

I don't think that would help because navigator.plugins would still advertise it. I'm looking into a fix too since I want to get builds going for rc1.

@diracdeltas
Copy link
Member

@bbondy it doesn't appear that navigator.plugins is advertising it

@bbondy
Copy link
Member

bbondy commented Jan 20, 2017

It's advertising it if it's installed pre the fix I'm doing.
Note that since we aren't using our placeholders anymore there is no indication to go to preferences, @srirambv pls post a follow up to get our placeholders back in for 0.13.something. (not 0.13.1 though).

@bbondy
Copy link
Member

bbondy commented Jan 20, 2017 •
edited
Loading

window renderer at least, probably the content setting hides it for content

@bbondy
Copy link
Member

bbondy commented Jan 20, 2017

with the fix I'm doing it won't even load the plugin anymore to get the info from it.

@bbondy bbondy closed this as completed in 054e7af Jan 20, 2017
@bbondy bbondy reopened this Jan 20, 2017
@bbondy
Copy link
Member

bbondy commented Jan 20, 2017

I think we'll try for an RC2 which has a less aggressive approach for better UX, so re-opening for now.

@diracdeltas
Copy link
Member

slightly-modified QA steps in aab8142

@BrendanEich BrendanEich mentioned this issue Jan 20, 2017
Open
@luixxiul luixxiul mentioned this issue Jan 21, 2017
Closed
NejcZdovc pushed a commit to NejcZdovc/browser-laptop that referenced this issue Jan 23, 2017
@bbondy @NejcZdovc
Require Flash to be explicitly enabled in preferences before it is us…
61718a7 
…able

Auditors: @diracdeltas

Fix brave#6739
NejcZdovc pushed a commit to NejcZdovc/browser-laptop that referenced this issue Jan 23, 2017
@diracdeltas @NejcZdovc
Change Flash context menu when Flash is not runnable
1910da6 
Fix brave#6739

Auditors: @bridiver @bbondy

Test Plan:
1. with flash disabled, go to http://homestarrunner.com/
2. right click on the placeholder
3. it should say 'Enable Flash in Preferences...', and clicking this should open preferences
4. enable flash
5. go back to http://homestarrunner.com/, right click on the placeholder again
6. choose 'allow for one week'. the flash should load.
7. disable flash
8. reload http://homestarrunner.com/. the flash should be blocked.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@diracdeltas diracdeltas

Labels
plugin/flash QA/checked-Win32 QA/checked-Win64 QA/test-plan-specified regression release-notes/exclude security
Projects
None yet
Milestone
0.13.0
Development

No branches or pull requests
4 participants
@diracdeltas @bbondy @luixxiul @srirambv