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

Launch first run dialog on macOS and linux #7700

Merged
merged 5 commits into from
Feb 2, 2021
Merged

Conversation

simonhong
Copy link
Member

@simonhong simonhong commented Jan 26, 2021

Resolves brave/brave-browser#12679

Development build doesn't launch this dialog.(https://github.com/brave/brave-core/pull/7700/files#diff-edfdf2e925a57fc30614d6d3ee529e49a5d9d29afe5eef326f1a83f09b456818R32)

On macOS:
image
On linux:
image

Submitter Checklist:

  • There is a ticket for my issue.
  • Used Github auto-closing keywords in the commit message.
  • Wrote a good PR/commit description
  • Added appropriate labels (QA/Yes or QA/No; release-notes/include or release-notes/exclude; OS/...) to the associated issue
  • Checked the PR locally: npm run test -- brave_browser_tests, npm run test -- brave_unit_tests, npm run lint, npm run gn_check, npm run tslint
  • Ran git rebase master (if needed).
  • Requested a security/privacy review as needed.

Reviewer Checklist:

  • New files have MPL-2.0 license header.
  • Adequate test coverage exists to prevent regressions
  • Major classes, functions and non-trivial code blocks are well-commented
  • Changes in component dependencies are properly reflected in gn
  • Code follows the style guide
  • Test plan is specified in PR before merging

After-merge Checklist:

Test Plan:

  1. Start Browser with clean profile and check first run dialog is launched.
  2. If default is checked, macOS asks about replacing default browser (linux doesn't ask more for replacing default browser)
  3. If reporting check box is set, p3a and crash report option are turned on
  4. If reporting check box is unset, p3a and crash report option are turned off.

@simonhong simonhong self-assigned this Jan 26, 2021
@simonhong simonhong force-pushed the first_run_dialog_macos branch 4 times, most recently from d658210 to b577920 Compare January 27, 2021 03:07
@simonhong simonhong changed the title Launch first run dialog on macOS Launch first run dialog on macOS and linux Jan 27, 2021
@simonhong simonhong force-pushed the first_run_dialog_macos branch from b577920 to 7626c35 Compare January 27, 2021 04:29
@simonhong simonhong changed the title Launch first run dialog on macOS and linux Launch first run dialog on macOS Jan 27, 2021
@simonhong simonhong marked this pull request as ready for review January 27, 2021 04:56
@simonhong simonhong requested a review from a team as a code owner January 27, 2021 04:56
@simonhong simonhong force-pushed the first_run_dialog_macos branch from 7626c35 to 9a4c9b5 Compare January 27, 2021 04:57
@simonhong simonhong force-pushed the first_run_dialog_macos branch 2 times, most recently from 48feb39 to 156afef Compare January 29, 2021 02:27
@simonhong simonhong changed the title Launch first run dialog on macOS Launch first run dialog on macOS and linux Jan 29, 2021
@simonhong simonhong requested a review from karenkliu January 29, 2021 05:51
@simonhong simonhong force-pushed the first_run_dialog_macos branch 2 times, most recently from 2283e59 to 2cdf368 Compare January 29, 2021 12:13
@iefremov
Copy link
Contributor

Implementation looks good, just want to double-check presence of P3A in this dialog, see Slack https://bravesoftware.slack.com/archives/CDNJ9SVUL/p1611956161037900

Copy link

@karenkliu karenkliu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! 👍

@simonhong simonhong force-pushed the first_run_dialog_macos branch from 10a74c5 to 4625915 Compare February 1, 2021 00:59
@simonhong simonhong requested a review from iefremov February 1, 2021 01:39
@simonhong
Copy link
Member Author

Kindly ping to @brave/patch-reviewers :)

Copy link
Collaborator

@mkarolin mkarolin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

++ on chromium_src

@simonhong simonhong force-pushed the first_run_dialog_macos branch from 4625915 to dfdb2d5 Compare February 2, 2021 00:05
@simonhong
Copy link
Member Author

AdBlockServiceTest.CosmeticFilteringProtect1p was failed only on Windows CI but it's unrelated with this PR.

@simonhong simonhong merged commit de3cccc into master Feb 2, 2021
@simonhong simonhong deleted the first_run_dialog_macos branch February 2, 2021 02:18
@simonhong simonhong added this to the 1.21.x - Nightly milestone Feb 2, 2021
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.

Set default browser first_run dialog on macOS and Linux
4 participants