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

Import default search engine #1064

Merged
merged 2 commits into from
Dec 13, 2018
Merged

Import default search engine #1064

merged 2 commits into from
Dec 13, 2018

Conversation

bsclifton
Copy link
Member

@bsclifton bsclifton commented Dec 11, 2018

Fixes brave/brave-browser#2421
Fixes brave/brave-browser#2415

Submitter Checklist:

  • Submitted a ticket for my issue if one did not already exist.
  • Used Github auto-closing keywords in the commit message.
  • Added/updated tests for this change (for new code or code which already has tests).
  • Verified that these changes build without errors on
    • Windows
    • macOS
    • Linux
  • Verified that these changes pass automated tests (npm test brave_unit_tests && npm test brave_browser_tests) on
    • Windows
    • macOS
    • Linux
  • Ran git rebase master (if needed).
  • Ran git rebase -i to squash commits (if needed).
  • Tagged reviewers and labelled the pull request as needed.
  • Request a security/privacy review as needed.
  • Add appropriate QA labels (QA/Yes or QA/No) to include the closed issue in milestone

Test Plan:

  1. Create a new Muon profile
    • open about:preferences#search
    • set default search engine to Bing
    • set Use DuckDuckGo by default for search in Private Tabs to true
    • set Use DuckDuckGo by default for search in Private Tabs with Tor to false
  2. Launch Brave Core
  3. Visit chrome://settings/importData
  4. Pick Brave (old) from the drop down and import (doesn't matter which items you pick)
  5. When import is finished, visit chrome://settings/searchEngines
  6. Verify default search engine is Bing
  7. Try a search in this window; verify it uses Bing
  8. Open a new Private Window and verify Search with DuckDuckGo is checked
  9. Try a search in this window; verify it uses DDG
  10. Close Private Windows + Open a NEW Private Window with Tor
  11. DDG logo and text should not show
  12. Try a search in this window; verify it uses Bing

Reviewer Checklist:

  • New files have MPL-2.0 license header.
  • Request a security/privacy review as needed.
  • Adequate test coverage exists to prevent regressions
  • Verify test plan is specified in PR before merging to source

…ents, allow import to finish.

Errors are still logged, but import calls `FinishLedgerImport` instead of `Cancel` (which may abort progress made)

Fixes brave/brave-browser#2421
browser/importer/brave_profile_writer.cc Outdated Show resolved Hide resolved
browser/importer/brave_profile_writer.cc Outdated Show resolved Hide resolved
browser/importer/brave_profile_writer.cc Outdated Show resolved Hide resolved
@bsclifton bsclifton changed the title WIP: Muon Importer fixes Import default search engine Dec 12, 2018
@bsclifton
Copy link
Member Author

Reduced scope of this PR, removed WIP label. Originally, I was going to put multiple fixes in here, but I think just these two are a great start.

I added a test plan and ran into issues when manually testing it. Basically, the private tab settings didn't import properly. Looking at this now...

@bsclifton
Copy link
Member Author

OK private windows now properly respect the DDG true/false value

I found a problem with Tor windows that I think is outside of the scope of this change. This PR will properly set kUseAlternativeSearchEngineProvider (which gets inherited by guest window). However, the engine doesn't seem to respect that value:
https://github.com/brave/brave-core/blob/master/browser/search_engines/tor_window_search_engine_provider_service.cc#L51-L67

@bsclifton
Copy link
Member Author

@mkarolin @darkdh ready for re-review 😄 👍

@darkdh
Copy link
Member

darkdh commented Dec 13, 2018

lgtm, please clean up commits then I will approve

- default search engine
- use DDG/Qwant for private tabs
- use DDG/Qwant for Tor tabs

Fixes brave/brave-browser#2415

NOTE: if private tabs in tor is not set, TorWindowSearchEngineProviderService::GetInitialSearchEngineProvider will properly set the default
@bsclifton
Copy link
Member Author

@darkdh commits squashed! 😄👌

@kjozwiak
Copy link
Member

@bsclifton uplift request to 0.58.x approved after deliberating with @srirambv & @rebron 👍 Please ensure that the correct labels are added/removed and the associated issue is moved to the correct milestone.

@bsclifton bsclifton merged commit c77e76b into master Dec 13, 2018
@bsclifton bsclifton deleted the bsc-importer-fixes branch December 13, 2018 16:27
bsclifton added a commit that referenced this pull request Dec 13, 2018
bsclifton added a commit that referenced this pull request Dec 13, 2018
bsclifton added a commit that referenced this pull request Dec 13, 2018
@bsclifton
Copy link
Member Author

master c77e76b
0.60.x bdd4dfd
0.59.x 5d5a411
0.58.x bcc8403

@bsclifton
Copy link
Member Author

0.58.x broke after the above cherry-pick; fixed with #1090

@bbondy bbondy added this to the 0.58.x - Release milestone Jan 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Brave Payments import failing can cause entire import to fail import search setting from muon
5 participants