-
Notifications
You must be signed in to change notification settings - Fork 965
Launch what's new page at major version update #17593
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
Conversation
7aa2ced
to
5d362b4
Compare
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.
chromium_src
++
void StartBraveWhatsNew(Browser* browser) { | ||
constexpr char kBraveWhatsNewURL[] = "https://brave.com/whats-new/"; | ||
// Load whats-new url in the first foreground tab. | ||
chrome::AddTabAt(browser, GURL(kBraveWhatsNewURL), 0, true); |
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.
why does open the url directly instead of fetching it like chromium?
} | ||
|
||
void RegisterLocalStatePrefs(PrefRegistrySimple* registry) { | ||
registry->RegisterDoublePref(prefs::kWhatsNewLastVersion, 0); |
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.
why are we creating our pref for this instead of using upstream kLastWhatsNewVersion
?
} | ||
|
||
// Returns 1.xx or 2.xx as double. | ||
absl::optional<double> GetCurrentBrowserVersion() { |
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.
why do we need all of this when the chromium code already does this on major version updates?
fix brave/brave-browser#28947
If griffin's target version is same with browser's current version,
browser will load whats-new url in the first foreground tab once per major version up.
Griffin will provide
target_major_version
params - https://github.com/brave/brave-variations/pull/544/files#diff-7ac2945ede68c73330363911678f6c9a2a6290b70e2dd170868295f65da7e872R1842When
whats_new::ShouldShowBraveWhatsNewForState()
returnstrue
,chrome://whats-new/
url is added to startup tabs list.For that loading that url, browser runs
whats_new::StartBraveWhatsNew()
instead of loading it directly. And
whats_new::StartBraveWhatsNew()
loadshttps://www.brave.com/whats-new/ in foreground first tab.
Resolves
Submitter Checklist:
QA/Yes
orQA/No
;release-notes/include
orrelease-notes/exclude
;OS/...
) to the associated issuenpm run test -- brave_browser_tests
,npm run test -- brave_unit_tests
wikinpm run lint
,npm run presubmit
wiki,npm run gn_check
,npm run tslint
git rebase master
(if needed)Reviewer Checklist:
gn
After-merge Checklist:
changes has landed on
Test Plan:
BraveWhatsNewTest.*
BraveWhatsNewBrowserTest.*
Manual test
Note: Maybe need some relaunching to
1.51
as it need to get trial data from griffinNote:
brave.whats_new.last_version
key in brave://local-state/ includes lastly shown whats-new version.