This repository has been archived by the owner on Dec 11, 2019. It is now read-only.
Removed "blank" option (about:blank !== about:newtab) #5335
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
already created issue at #5334 |
|
updated 😄 |
Fixes #5334 Definitely my mistake folks- I had forgotten about the URL bar getting focus and didn't consider that the change broke tests also :( This change: - finalizes the name of the setting (the previous one had "TEST" in it) - defaults unknown URLs to about:newtab (which makes sense, since those have URL bar focus) - removes blank as an option (since it was going to be removed when we shipped) If we need to disable the NEW about:newtab screen for 0.12.8, we can just return an empty div in `js/about/newtab.js` Auditors: @diracdeltas, @bbondy Test Plan: 1. Launch Brave and open a new tab 2. New tab should be about:newtab 3. Preferences > General should no longer show "blank" 4. Tests should work again
|
lgtm, thanks |
This was referenced Nov 2, 2016
This was referenced Nov 4, 2016
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #5334
Definitely my mistake folks- I had forgotten about the URL bar getting focus and didn't consider that the change broke tests also☹️
This change:
If we need to disable the NEW about:newtab screen for 0.12.8, we can just return an empty div in
js/about/newtab.jsAuditors: @diracdeltas, @bbondy
Test Plan: