This repository has been archived by the owner on Dec 11, 2019. It is now read-only.
"Select its parent tab" setting now works as expected #9400
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
|
Requires brave/muon#214 before this can be merged |
- Don't consider if tab was active or not - Validate tabId before returning requires brave/muon#214 Fixes #9395 Auditors: @bridiver, @BrendanEich
|
ping @bridiver |
bridiver
reviewed
Jun 15, 2017
app/common/state/tabState.js
Outdated
| const openerTabId = tabState.getTabPropertyByTabId(state, tabId, 'openerTabId', tabState.TAB_ID_NONE) | ||
| if (openerTabId !== tabState.TAB_ID_NONE) { | ||
| // Validate that tabId exists | ||
| const index = getTabInternalIndexByTabId(state, openerTabId) |
There was a problem hiding this comment.
I would probably use getByTabId here and it already handles TAB_ID_NONE by returning null. getTabInternalIndex is just a temporary hack until we can re-organize the tab state so best to limit its usage to the smallest set of methods possible
bridiver
reviewed
Jun 15, 2017
app/browser/reducers/tabsReducer.js
Outdated
| @@ -51,15 +51,14 @@ const updateActiveTab = (state, closeTabId) => { | |||
| case tabCloseAction.PARENT: | |||
| { | |||
| const openerTabId = tabState.getOpenerTabId(state, closeTabId) | |||
| const lastActiveTabId = tabState.getLastActiveTabId(state, windowId) | |||
| if (openerTabId === lastActiveTabId) { | |||
| if (openerTabId && openerTabId !== tabState.TAB_ID_NONE) { | |||
There was a problem hiding this comment.
is the null check here necessary?
bridiver
approved these changes
Jun 15, 2017
|
nits fixed 😄 merging |
bsclifton
added a commit
that referenced
this pull request
Jun 16, 2017
"Select its parent tab" setting now works as expected
bsclifton
added a commit
that referenced
this pull request
Jun 16, 2017
"Select its parent tab" setting now works as expected
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.
"Select its parent tab" setting now works as expected
Fixes #9395
Auditors: @bridiver, @BrendanEich
Submitter Checklist:
git rebase -ito squash commits (if needed).Reviewer Checklist:
Tests