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

Reintroduce the whitelist for websites linking to flash download page #14269

Merged
merged 1 commit into from
May 30, 2018

Conversation

Slava
Copy link
Contributor

@Slava Slava commented May 29, 2018

Fixes the #13500 regression, does not add a regression test. It is not entirely clear to me if a regression test with a google's result page of "flash test" would be very useful but let me know what you think.

Submitter Checklist:

  • Submitted a ticket for my issue if one did not already exist.
  • Used Github auto-closing keywords in the commit message.
  • Added/updated tests for this change (for new code or code which already has tests).
  • Ran git rebase -i to squash commits (if needed).
  • Tagged reviewers and labelled the pull request as needed.
  • Request a security/privacy review as needed. (Ask a Brave employee to help if you cannot access this document.)

Test Plan:

Manually tested the popup behavior on google and a test page

To test:

  • install and enable flash, restart the browser
  • search "flash test" in google and click the link to the flash download page on adobe website (likely first search result)
  • observe that there is no pop-up prompt to allow flash on the google page (the adobe page will have the prompt because it runs an ad with flash)
  • go to the test page: https://github.com/Slava/resume/issues/1
  • click the same link to the download page
  • observe that the browser didn't change the page, but instead prompted to enable flash on that page

 ## Reviewer Checklist:

  • Request a security/privacy review as needed if one was not already requested.

Tests

  • Adequate test coverage exists to prevent regressions
  • Tests should be independent and work correctly when run individually or as a suite ref
  • New files have MPL2 license header

@diracdeltas diracdeltas self-requested a review May 29, 2018 23:29

function shouldIntercept(location) {
if (!location)
return true;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[minor] please use curly braces around the function body and no semicolon

this would normally be caught by npm run lint but the linter excludes content scripts :(

@diracdeltas diracdeltas added this to the 0.24.x (Nightly Channel) milestone May 29, 2018
@diracdeltas
Copy link
Member

master / 0.24.x: 22c17a4

bsclifton pushed a commit that referenced this pull request Jul 31, 2018
Reintroduce the whitelist for websites linking to flash download page
@bsclifton
Copy link
Member

Uplifted to 0.23.x with 5ab5980

# for free to subscribe to this conversation on GitHub. Already have an account? #.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants