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

Fix Ads locale on Windows #4172

Closed
tmancey opened this issue Apr 19, 2019 · 5 comments · Fixed by brave/brave-core#2285
Closed

Fix Ads locale on Windows #4172

tmancey opened this issue Apr 19, 2019 · 5 comments · Fixed by brave/brave-core#2285

Comments

@tmancey
Copy link
Contributor

tmancey commented Apr 19, 2019

Description

Ads locale returns incorrect region on Windows

Steps to Reproduce

  1. Enable Ads

Actual result:

Ads is not supported for US, UK, France, Germany and Canadian regions

Expected result:

Ads should be supported for US, UK, France, Germany and Canadian regions

Reproduces how often:

Easily reproduced

Brave version (brave://version info)

0.65.70

Version/Channel Information:

  • Can you reproduce this issue with the current release? YES
  • Can you reproduce this issue with the beta channel? YES
  • Can you reproduce this issue with the dev channel? YES
  • Can you reproduce this issue with the nightly channel? YES

Other Additional Information:

  • Does the issue resolve itself when disabling Brave Shields? N/A
  • Does the issue resolve itself when disabling Brave Rewards? N/A
  • Is the issue reproducible on the latest version of Chrome? N/A

Miscellaneous Information:

@tmancey tmancey added this to the 0.66.x - Nightly milestone Apr 19, 2019
@tmancey tmancey self-assigned this Apr 19, 2019
@jsecretan jsecretan added priority/P1 A very extremely bad problem. We might push a hotfix for it. release/blocking labels Apr 19, 2019
tmancey added a commit to brave/brave-core that referenced this issue Apr 19, 2019
tmancey added a commit to brave/brave-core that referenced this issue Apr 19, 2019
tmancey added a commit to brave/brave-core that referenced this issue Apr 19, 2019
tmancey added a commit to brave/brave-core that referenced this issue Apr 19, 2019
tmancey added a commit to brave/brave-core that referenced this issue Apr 19, 2019
tmancey added a commit to brave/brave-core that referenced this issue Apr 19, 2019
tmancey added a commit to brave/brave-core that referenced this issue Apr 19, 2019
@GeetaSarvadnya
Copy link

GeetaSarvadnya commented Apr 22, 2019

Verification passed on

Brave 0.63.46 Chromium: 74.0.3729.91 (Official Build) (64-bit)
Revision 03844ed83e02b8add3f4b9cb859a7108d55b2e4d-refs/branch-heads/3729@{#860}
OS Windows 10 OS Build 17134.523
  • Verified below combinations (IP Address+OS Region+OS Locale) of scenarios on Windows 10 x64

    • US IP Address(VPN)+Region(US)+Locale(English United States) -Adds are Enabled
    • US IP Address(VPN)+Region(US)+Locale(English India)-Adds are disabled &catalog is NOT empty
    • US IP Address(VPN)+Region(US)+Locale(English United Kingdom) -Adds are Enabled
    • India IP Address+Region(US)+Locale(English United States)-Adds are disabled &catalog is empty
    • India IP Address+Region(India)+Locale(English India)-Adds are Disabled and catalog is empty
    • US IP Address(VPN)+Region(UK)+Locale(English United Kingdom) -Adds are Enabled
    • US IP Address(VPN)+Region(Germany)+Locale(English United Kingdom) -Adds are Enabled
    • US IP Address(VPN)+Region(Canada)+Locale(English Canada) -Adds are Enabled
  • Logged "Sorry! Ads are not supported for this region" message will not be shown in some criteria #4187

@GeetaSarvadnya
Copy link

GeetaSarvadnya commented Apr 22, 2019

@terry In one of the scenario even though Ads are disabled catalog is not empty is this expected?

US IP Address(VPN)+Region(US)+Locale(English India)-Adds are disabled which is expected as one of the criteria fails (locale) but, the catalog is NOT empty is this expected?

@tmancey
Copy link
Contributor Author

tmancey commented Apr 22, 2019

Region/locale are used currently to determine so this is correct based upon the code. Please check with Jimmy for confirmation too. Thanks

@GeetaSarvadnya
Copy link

@jsecretan Can you clarify the above query, please?

@GeetaSarvadnya
Copy link

GeetaSarvadnya commented Apr 22, 2019

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

Successfully merging a pull request may close this issue.

5 participants