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

Asynchronous Popups are Sometimes Permitted #4139

Closed
jonathansampson opened this issue Sep 21, 2016 · 1 comment
Closed

Asynchronous Popups are Sometimes Permitted #4139

jonathansampson opened this issue Sep 21, 2016 · 1 comment
Labels
needs-investigation A bug not 100% confirmed/fixed that needs QA to better audit. sec-low security stale

Comments

@jonathansampson
Copy link
Collaborator

jonathansampson commented Sep 21, 2016

Did you search for similar issues before submitting this one?

Yes.

Describe the issue you encountered:

Asynchronous popups should be blocked, but are sometimes permitted. The issue appears to be one of timing.

The behavior can be observed by navigating here, and quickly clicking the document a few times.

Expected behavior:

Asynchronous popups are blocked.

  • Platform (Win7, 8, 10? macOS? Linux distro?): Windows 10
  • Brave Version: 0.12.1
  • Steps to reproduce:
    1. Trigger a call to window.open from setTimeout
    2. Note that some calls will result in a new window, while others will not
  • Any related issues: Potentially, a few.
@bridiver
Copy link
Collaborator

bridiver commented Sep 21, 2016

the "user gesture" status of these popups is sometimes interfered with by a setInterval call in one of the content scripts. I can reproduce the same behavior on chrome by adding a setInterval call on the page. The interval is used instead of mutation observers which were causing performance issues in various benchmarks, but there are alternatives for all of them:

  1. flash listener placeholder - we can get notifications of flash objects added to the dom from ContentSettingsClient::allowPlugins
  2. flash adobe installer notification - I think we can use a redirect to a javascript data uri or possibly a custom protocol handler to trigger the same notification
  3. password manager - move to electron as planned
    We can also reduce the problem by increasing the interval time

@luixxiul luixxiul added the needs-investigation A bug not 100% confirmed/fixed that needs QA to better audit. label Oct 6, 2016
@bsclifton bsclifton added this to the Triage Backlog milestone Nov 27, 2017
@bsclifton bsclifton removed this from the Triage Backlog milestone Aug 26, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
needs-investigation A bug not 100% confirmed/fixed that needs QA to better audit. sec-low security stale
Projects
None yet
Development

No branches or pull requests

5 participants