Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fixes ads on yt and gets rid of csp error #8071

Merged
merged 1 commit into from Feb 25, 2021
Merged

Conversation

SergeyZhukovsky
Copy link
Member

@SergeyZhukovsky SergeyZhukovsky commented Feb 25, 2021

Resolves brave/brave-browser#14359

Submitter Checklist:

  • I confirm that no security/privacy review is needed, or that I have requested one
  • There is a ticket for my issue
  • Used Github auto-closing keywords in the PR description above
  • Wrote a good PR/commit description
  • Added appropriate labels (QA/Yes or QA/No; release-notes/include or release-notes/exclude; OS/...) to the associated issue
  • Checked the PR locally: npm run test -- brave_browser_tests, npm run test -- brave_unit_tests, npm run lint, npm run gn_check, npm run tslint
  • Ran git rebase master (if needed)

Reviewer Checklist:

  • A security review is not needed, or a link to one is included in the PR description
  • New files have MPL-2.0 license header
  • Adequate test coverage exists to prevent regressions
  • Major classes, functions and non-trivial code blocks are well-commented
  • Changes in component dependencies are properly reflected in gn
  • Code follows the style guide
  • Test plan is specified in PR before merging

After-merge Checklist:

Test Plan:

described in the original issue

Copy link
Collaborator

@antonok-edm antonok-edm left a comment

Choose a reason for hiding this comment

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

I can't speak for the isolated world stuff, but the scriptlet injection method looks good to me 👍

@SergeyZhukovsky
Copy link
Member Author

post-init failed due to audit problem which is not related to the PR

@stephendonner
Copy link
Collaborator

stephendonner commented Feb 26, 2021

Looks good using:

Brave 1.23.5 Chromium: 89.0.4389.58 (Official Build) nightly (x86_64)
Revision 1a139f28ecc27719439e37c6b1533cee999cb802-refs/branch-heads/4389@{#1134}
OS macOS Version 11.2.2 (Build 20D80)

Steps from brave/brave-browser#14359

  1. Sign in to YouTube using non-Premium YouTube/Google account
  2. Open a video link https://www.youtube.com/watch?v=xXMW9VNMPT4
  3. Make sure it plays and pick some other video from a right panel
  4. Make sure second video plays, click Back arrow on a browser panel to come back to the first video

Verified that upon clicking Back I didn't get any ad or ad overlay:

youtube-nightly

I did notice that going back and forward a few times triggers the ad overlay, but in talking with @SergeyZhukovsky it sounds like a separate issue to be investigated/clarified further, and not part of the original issue here (which only concerns an initial Back action).

Confirmed parity with

Brave 1.20.110 Chromium: 88.0.4324.192 (Official Build) (x86_64)
Revision 31b458a18f133db9203eb5a5dd6552de0716dda3-refs/branch-heads/4324_182@{#6}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants