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

Ad notifications sometimes presented to user when Ads toggle is off - follow up to 2972 #3408

Closed
LaurenWags opened this issue Feb 18, 2019 · 6 comments

Comments

@LaurenWags
Copy link
Member

LaurenWags commented Feb 18, 2019

Description

Follow up to #2972

Found an additional scenario when Ads are presented to the user when Ads toggle is off in the UI.

If you toggle Rewards off, close and relaunch Brave, then toggle Rewards on, UI will show Ads as off, but in fact, Ads Service is running (you are able to see Ads Service messages in terminal if you've launched with the correct flags) and you will be presented with Ad notifications.

Seems as though there are two problems here:

  1. When Rewards are toggled back on after browser restart, Ads are showing as off in the UI.
  2. Ads Service isn't respecting UI toggle which is set to off.

Steps to Reproduce

  1. Launch Brave with a clean profile - I also used these flags --enable-logging=stderr --vmodule=brave_ads=3 --brave-ads-debug
  2. Enable Rewards, Ads are enabled by default as expected.
  3. Visit some sites - you will see AdsService messages in terminal as expected since Ads are on.
  4. Toggle Rewards off.
  5. Visit some sites, you will not see AdsService messages in terminal as expected since Ads are off.
  6. Toggle Rewards back on.
  7. Visit some sites, you will see AdsService messages in terminal as expected since Ads are on.
  8. Toggle Rewards off.
  9. Close Brave.
  10. Relaunch Brave (I used the same flags as before)
  11. Toggle Rewards on again.

Actual result:

Ads toggle is off in UI, but if you visit pages you will see AdsService messages in the terminal. After a bit of browsing you will also start to get Ad notifications.

screen shot 2019-02-18 at 4 03 44 pm

Expected result:

Since Ads were on before Rewards was disabled, I'd expect toggling Rewards on to turn on Ads. However, if Ads toggle is off in the UI, then I should not be presented with Ad notifications.

Reproduces how often:

easily

Brave version (brave://version info)

Brave 0.60.28 Chromium: 72.0.3626.96 (Official Build) beta(64-bit)
Revision 84098ee7ef8622a9defc2ef043cd8930b617b10e-refs/branch-heads/3626@{#836}
OS Mac OS X

Reproducible on current release: no, Ads are not available on 0.59.x

  • Does it reproduce on brave-browser dev/beta builds? reproduced on beta 0.60.x

Website problems only:

  • Does the issue resolve itself when disabling Brave Shields? n/a
  • Is the issue reproducible on the latest version of Chrome? n/a

Additional Information

cc @brave/legacy_qa

@btlechowski
Copy link

Reproduced on Windows 7

Brave 0.60.29 Chromium: 72.0.3626.96 (Official Build) beta (64-bit)
Revision 84098ee7ef8622a9defc2ef043cd8930b617b10e-refs/branch-heads/3626@{#836}
OS Windows 7

@LaurenWags LaurenWags added the priority/P2 A bad problem. We might uplift this to the next planned release. label Feb 19, 2019
@jsecretan jsecretan added priority/P1 A very extremely bad problem. We might push a hotfix for it. and removed priority/P2 A bad problem. We might uplift this to the next planned release. labels Feb 26, 2019
@ryanml ryanml self-assigned this Mar 4, 2019
@ryanml
Copy link
Contributor

ryanml commented Mar 4, 2019

Fixed via: brave/brave-core#1640

(Note, this should be re-verified on the latest version of 0.60) cc: @LaurenWags @jsecretan

@kjozwiak
Copy link
Member

kjozwiak commented Mar 7, 2019

@ryanml we just released 0.60.48 today which bumped the CR from 72.0.3626.119 to 72.0.3626.121. Was there any work that needed to go into that build that's related to this? Or did 0.60.48 just need a recheck?

@ryanml
Copy link
Contributor

ryanml commented Mar 7, 2019

@kjozwiak 0.60.48 should just need a recheck

@kjozwiak
Copy link
Member

kjozwiak commented Mar 7, 2019

@RyanJarv sounds good 👍 I'll give this a check tomorrow. @LaurenWags mind giving this another recheck on 0.60.48 as well?

@LaurenWags
Copy link
Member Author

Verified passed with

Brave 0.60.48 Chromium: 72.0.3626.121 (Official Build) (64-bit)
Revision da3787ba355f18db7db52abf75c42afb408d656f-refs/branch-heads/3626@{#883}
OS Mac OS X
  • Verified launching with /Applications/Brave\ Browser.app/Contents/MacOS/Brave\ Browser --enable-logging=stderr --vmodule=brave_ads=3 --log-level=0, enabling Rewards, and browsing did not show any AdService Event Log messages in terminal
  • Verified turning Rewards off and browsing still did not show any AdService Event Log messages in terminal
  • Verified closing/relaunching Brave, enabling Rewards again, and browsing did not show any AdService Event Log messages in terminal
  • Verified to way to toggle Ads on in UI
  • Verified no Ad notifications were presented while browsing

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

No branches or pull requests

7 participants