-
Notifications
You must be signed in to change notification settings - Fork 870
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
Update search engine defaults for DE / AU / NZ / IE #3591
Conversation
0112e12
to
8decce1
Compare
8decce1
to
60e3672
Compare
chromium_src/components/search_engines/brave_template_url_service_util_unittest.cc
Outdated
Show resolved
Hide resolved
chromium_src/components/search_engines/brave_template_url_prepopulate_data_unittest.cc
Show resolved
Hide resolved
chromium_src/components/search_engines/brave_template_url_service_util_unittest.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.
LGTM with small nits
chromium_src/components/search_engines/template_url_prepopulate_data.cc
Outdated
Show resolved
Hide resolved
e09482b
to
819a68b
Compare
Looks like just a lint failure - I can fix this real quick: |
819a68b
to
9be289f
Compare
chromium_src/components/search_engines/brave_template_url_prepopulate_data_unittest.cc
Outdated
Show resolved
Hide resolved
chromium_src/components/search_engines/brave_template_url_service_util_unittest.cc
Outdated
Show resolved
Hide resolved
chromium_src/components/search_engines/brave_template_url_service_util_unittest.cc
Outdated
Show resolved
Hide resolved
chromium_src/components/search_engines/template_url_prepopulate_data.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.
LGTM
Has one failing lint (swear I fixed all in a previous commit), but otherwise looks good. I'll push a quick fix for the lint and then merge (as this is approved and CI has otherwise passed) |
Update search engine defaults for DE / AU / NZ / IE
Update search engine defaults for DE / AU / NZ / IE
Fixes brave/brave-browser#6421 Problem was regular DDG not found in the entries returned (since locale specific was returned instead). Besides adding other entries, this also puts a dcheck in place to ensure an engine gets set (otherwise, the error thrown is no immediately clear and a bit hard to trace). (bug unintentionally introduced with #3591)
Fixes brave/brave-browser#6187
Submitter Checklist:
npm run lint
)git rebase master
(if needed).git rebase -i
to squash commits (if needed).Test Plan:
See https://github.com/brave/internal/issues/647
Reviewer Checklist:
After-merge Checklist:
changes has landed on.