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

Additional NTP is needed to show NTP SI in opt-in state when initial_count_to_branded_wallpaper is zero #31951

Closed
btlechowski opened this issue Jul 28, 2023 · 4 comments

Comments

@btlechowski
Copy link

btlechowski commented Jul 28, 2023

Follow up to #30974

In opt-in state user needs to open one more NTP compared to opt-out state when initial_count_to_branded_wallpaper=0

Steps to Reproduce

opt-in

  1. Clean profile
  2. Run Brave with
--enable-features=BraveNTPBrandedWallpaper:initial_count_to_branded_wallpaper/0/count_to_branded_wallpaper/3,ShouldAlwaysTriggerBraveNewTabPageAdEvents
  1. Enable rewards and ads
  2. Make sure all ads components are downloaded, including NTP SI.
  3. Open NTP

Actual result:

NTP SI is not shown on 1st NTP
NTP SI is shown on 2nd NTP in opt-in state.
In opt-in state user needs to open one more NTP compared to opt-out state

Note: it behaves as if initial_count_to_branded_wallpaper = 1

Expected result:

NTP SI is shown on 1st NTP

Reproduces how often:

Easily reproduced

Brave version (brave://version info)

Brave 1.57.27 Chromium: 115.0.5790.114 (Official Build) beta (64-bit)
Revision 18977fb43f693d5a8deeb48bd1cfa52651d3f73e
OS Ubuntu 18.04 LTS

cc @tmancey @aseren

@tmancey
Copy link
Contributor

tmancey commented Oct 3, 2023

There were no plans to set this to zero. Unless this is something that QA would need? Thanks

@tmancey tmancey closed this as completed Oct 3, 2023
@btlechowski btlechowski changed the title Additional NTP is need to show NTP SI in opt-in state when initial_count_to_branded_wallpaper is zero Additional NTP is needed to show NTP SI in opt-in state when initial_count_to_branded_wallpaper is zero Oct 8, 2023
@btlechowski
Copy link
Author

btlechowski commented Oct 11, 2023

Reopened as dev team changed initial_count_to_branded_wallpaper to 0 brave/brave-core#19823 (review) cc @petemill

@petemill
Copy link
Member

Initial count has not been modified in that PR. Further, even if it was 0, then that PR changed what initial count means. It now means count after data is received. When you start a fresh profile there is no SI data to show. It loads from remote component. So, this is working as expected.

@petemill petemill closed this as not planned Won't fix, can't repro, duplicate, stale Oct 12, 2023
@btlechowski
Copy link
Author

The point of this issue that when ads components are downloaded and initialized then opening first NTP should be NTP SI. This is not the case right now.

If we ever want to change initial_count_to_branded_wallpaper=0, it will not work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

No branches or pull requests

3 participants