Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Fixes #2913: Closing an inactive tab should not change the active tab #2923

Merged
merged 2 commits into from
Aug 6, 2016
Merged

Fixes #2913: Closing an inactive tab should not change the active tab #2923

merged 2 commits into from
Aug 6, 2016

Conversation

willy-b
Copy link
Contributor

@willy-b willy-b commented Aug 3, 2016

@bsclifton
Copy link
Member

bsclifton commented Aug 5, 2016

I pulled this code down locally and it does fix the case outlined in the issue... However, I was able to reproduce a scenario where it doesn't work though. Can you please help look into why?

Steps

  1. Open https://www.wikipedia.org/ in tab 1
  2. Follow any link on that page, using middle click to create a new tab (but not sending focus there). This is tab 2
  3. Open a new tab, tab 3, by clicking the plus and then navigating to whatever site you like
  4. Set the active focus to tab 1
  5. Close tab 3
  6. Focus is now on tab 2 😦

NOTE: in my example, I had two pinned tabs. I tried unpinning them and the behavior did not change (it still failed, per the above steps)

- windowStore.doAction should pass FrameStateUtil.removeFrame the _active frame key_ in activeFrameKey arg, not the _closed frame key_ (fixed)
- FrameStateUtil.removeFrame should only set the active frame to the parent of the closed frame IFF the frame closed _was active_ (fixed)
@willy-b
Copy link
Contributor Author

willy-b commented Aug 5, 2016

Thanks @bsclifton for catching that.

I just pushed another commit which should fix the case you pointed out, and the separate case where the 3rd tab is the child of the 2nd tab.

@bsclifton
Copy link
Member

Works great! Thanks for taking the time to dig in and get the parentFrameKey fix in there 😄

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants