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 default search logic for Tor window and guest window #3738

Merged
merged 3 commits into from
Oct 21, 2019

Conversation

bsclifton
Copy link
Member

@bsclifton bsclifton commented Oct 17, 2019

Fixes brave/brave-browser#6476

Specifically fixes:

  • crash that was happening when expected search (regular DDG) was not found
  • toggle (slider) status for DDG on guest window

Submitter Checklist:

Test Plan:

I ran through the following:

Germany

Search in...

  • regular window
  • private window
  • click toggle on private window and search
  • Tor window
  • guest window
  • toggle for guest window DDG is checked (if DDG default)

Australia

Search in...

  • regular window
  • private window
  • click toggle on private window and search
  • Tor window
  • guest window
  • toggle for guest window DDG is checked (if DDG default)

New Zealand

Search in...

  • regular window
  • private window
  • click toggle on private window and search
  • Tor window
  • guest window
  • toggle for guest window DDG is checked (if DDG default)

Ireland

Search in...

  • regular window
  • private window
  • click toggle on private window and search
  • Tor window
  • guest window
  • toggle for guest window DDG is checked (if DDG default)

USA

Search in...

  • regular window
  • private window
  • click toggle on private window and search
  • Tor window
  • guest window
  • toggle for guest window DDG is NOT checked (DDG is not default)

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

After-merge Checklist:

  • The associated issue milestone is set to the smallest version that the
    changes has landed on.
  • All relevant documentation has been updated.

@bsclifton bsclifton added this to the 0.73.x - Nightly milestone Oct 17, 2019
@bsclifton bsclifton self-assigned this Oct 17, 2019
@bsclifton bsclifton force-pushed the bsc-search-engine-fixes branch from 0749e00 to e854bcc Compare October 17, 2019 23:06
@bsclifton bsclifton changed the title Bsc search engine fixes Fix default search logic for Tor window and guest window Oct 17, 2019
@bsclifton bsclifton marked this pull request as ready for review October 17, 2019 23:47
@bsclifton bsclifton requested a review from bridiver as a code owner October 17, 2019 23:47
@bsclifton bsclifton requested a review from mkarolin October 17, 2019 23:48
@bsclifton bsclifton force-pushed the bsc-search-engine-fixes branch from a4208d5 to 487ab5f Compare October 18, 2019 04:47
@simonhong
Copy link
Member

I think we can do some cleanup TorWindow... and GuestWindow....
When I implemented them at first, each one's provider could be changed via its settings page. but we don't provide settings page of tor window or guest window.
If we don't have a plan to add it again, I think many codes could be deleted.

@bsclifton
Copy link
Member Author

bsclifton commented Oct 18, 2019

I think we can do some cleanup TorWindow... and GuestWindow....
When I implemented them at first, each one's provider could be changed via its settings page. but we don't provide settings page of tor window or guest window.
If we don't have a plan to add it again, I think many codes could be deleted.

That would be a good follow up for sure - not sure if that should be done in this PR (this needs to go back to 0.70, for C78)

How do things look otherwise, @simonhong? 😄

@bsclifton bsclifton requested a review from simonhong October 18, 2019 16:44
Fixes brave/brave-browser#6476

Specifically, fixes the engine when a previously set engine is no longer available.

For example, in DE the engine used to be DDG (501), but was changed in
the prepopulated list to DDG_DE(516). The value in the preferences will
not be able to be used to get a valid engine (and cause a crash). This
change checks if the currently selected engine is available and if not
resets the value to the currently preset default.
@bsclifton bsclifton force-pushed the bsc-search-engine-fixes branch from 487ab5f to f07cdf2 Compare October 20, 2019 23:39
@bsclifton
Copy link
Member Author

Given the manual testing done + full CI passing (and the need for a build soon), I'm going to merge this one 👍

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.

Browser crash when Tor/Guest/Private window is opened in some locales - follow up to 6421
2 participants