-
Notifications
You must be signed in to change notification settings - Fork 920
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
[ads] Suppress RichNTT if brave://settings/content/javascript is disabled #27783
base: master
Are you sure you want to change the base?
Conversation
9eb2be7
to
9532f22
Compare
5bc6b3e
to
d2e1a66
Compare
7f0b621
to
811b39e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
iOS++
811b39e
to
87e1e3c
Compare
components/ntp_background_images/browser/view_counter_service.cc
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i would add a manual test plan that toggling brave://settings/content/javascript on and off works as expected but otherwise lgtm
Just finishing adding the tests, and thank you. |
components/ntp_background_images/browser/ntp_sponsored_rich_media_source.h
Outdated
Show resolved
Hide resolved
cbc9b69
to
8511755
Compare
4a981b3
to
6f81487
Compare
6f81487
to
aabcae4
Compare
[puLL-Merge] - brave/brave-core@27783 Here's my analysis of this pull request: DescriptionThis PR modifies the Brave browser codebase to respect JavaScript content settings when displaying sponsored rich media content in new tab takeovers. It adds logic to check if JavaScript is enabled before allowing rich media wallpapers, while still allowing image-based wallpapers regardless of JavaScript settings. The PR also includes significant refactoring of related components and test improvements. ChangesChangesBy filename:
sequenceDiagram
participant User
participant Browser
participant ContentSettings
participant AdsService
participant Database
User->>Browser: Opens New Tab
Browser->>ContentSettings: Check JavaScript settings
ContentSettings->>AdsService: Allow/Block rich media
AdsService->>Database: Query eligible wallpapers
Database->>AdsService: Return filtered wallpapers
AdsService->>Browser: Return appropriate wallpaper
Browser->>User: Display wallpaper
Security Hotspots
|
Resolves brave/brave-browser#43655
Submitter Checklist:
QA/Yes
orQA/No
;release-notes/include
orrelease-notes/exclude
;OS/...
) to the associated issuenpm run test -- brave_browser_tests
,npm run test -- brave_unit_tests
wikinpm run presubmit
wiki,npm run gn_check
,npm run tslint
git rebase master
(if needed)Reviewer Checklist:
gn
After-merge Checklist:
changes has landed on
Test Plan:
Confirm rich media NTTs are shown if brave://settings/content/javascript is enable and not shown if disabled.