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

[ads] Should not fetch geo and catalog on iOS when Brave Rewards/News disabled #39621

Closed
aseren opened this issue Jul 8, 2024 · 3 comments · Fixed by brave/brave-core#24570
Closed
Assignees
Labels
bug feature/ads OS/iOS Fixes related to iOS browser functionality priority/P1 A very extremely bad problem. We might push a hotfix for it. QA Pass - iPhone QA/Yes release-notes/include

Comments

@aseren
Copy link

aseren commented Jul 8, 2024

Description

Following Brave News chromium preferences are always set to true because iOS uses it's own set of native preferences:

brave_news::prefs::kBraveNewsOptedIn
brave_news::prefs::kNewTabPageShowToday

https://github.com/brave/brave-core/blob/master/ios/browser/api/ads/brave_ads.mm#L1531

This was working as expected until ShouldAlwaysRunBraveAdsService feature is enabled for iOS where we start brave ads when rewards and news are disabled. To fix the issue we need to properly set Brave News chromium preferences basing on Brave News native preference values for iOS.

Following preferences should be also properly set:

ntp_background_images::prefs::kNewTabPageShowBackgroundImage
ntp_background_images::prefs::kNewTabPageShowSponsoredImagesBackgroundImage
@aseren aseren added bug feature/ads OS/iOS Fixes related to iOS browser functionality labels Jul 8, 2024
@aseren aseren added this to Ads Jul 8, 2024
@github-project-automation github-project-automation bot moved this to New issues in Ads Jul 8, 2024
@tmancey tmancey added priority/P1 A very extremely bad problem. We might push a hotfix for it. QA/Yes release-notes/exclude release-notes/include and removed release-notes/exclude labels Jul 8, 2024
@tmancey tmancey moved this from New issues to In progress in Ads Jul 8, 2024
@aseren aseren moved this from In progress to Review in Ads Jul 9, 2024
@github-project-automation github-project-automation bot moved this from Review to Done in Ads Jul 10, 2024
@brave-builds brave-builds added this to the 1.69.x - Nightly milestone Jul 10, 2024
@aseren
Copy link
Author

aseren commented Jul 12, 2024

These settings will be properly handled within the issue: #39658

ntp_background_images::prefs::kNewTabPageShowBackgroundImage
ntp_background_images::prefs::kNewTabPageShowSponsoredImagesBackgroundImage

@kjozwiak
Copy link
Member

The above requires 1.68.122 or higher for 1.68.x verification 👍

@btlechowski
Copy link

Verified on Brave 1.68.128 on iPhone 13 Pro Max

Verified test plan from brave/brave-core#24570

Testcase 1

image

Brave Ads started and successfully initialized

info	02:52:49.754250+0200	Client	[ads] Successfully initialized ads

there is NO network request to https://geo.ads.bravesoftware.com/v1/getstate endpoint
there is NO network request to https://static.ads.bravesoftware.com/v9/catalog endpoint

there is network request to https://geo.ads.bravesoftware.com/v1/getstate endpoint after enabling Brave News

info	19:44:06.662122+0200	Client	[ads] URL Request:
  URL: https://geo.ads.bravesoftware.com/v1/getstate
  Method: kGet

there is network request to https://static.ads.bravesoftware.com/v9/catalog endpoint after enabling Brave News

info	19:44:06.662122+0200	Client	[ads] URL Request:
  URL: https://geo.ads.bravesoftware.com/v1/getstate
  Method: kGet

Testcase 2

image

Brave Ads started and successfully initialized

info	19:47:22.770364+0200	Client	[ads] Successfully initialized ads

there is NO network request to https://geo.ads.bravesoftware.com/v1/getstate endpoint
there is NO network request to https://static.ads.bravesoftware.com/v9/catalog endpoint

there is network request to https://geo.ads.bravesoftware.com/v1/getstate endpoint after enabling Brave News

info	19:47:22.770468+0200	Client	[ads] URL Request:
  URL: https://static.ads.bravesoftware.com/v9/catalog
  Method: kGet

there is network request to https://static.ads.bravesoftware.com/v9/catalog endpoint after enabling Brave News

info	19:47:22.770997+0200	Client	[ads] URL Request:
  URL: https://geo.ads.bravesoftware.com/v1/getstate
  Method: kGet

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug feature/ads OS/iOS Fixes related to iOS browser functionality priority/P1 A very extremely bad problem. We might push a hotfix for it. QA Pass - iPhone QA/Yes release-notes/include
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

5 participants