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

Closing tab switches to the second tab from root tab #11981

Closed
srirambv opened this issue Nov 15, 2017 · 11 comments
Closed

Closing tab switches to the second tab from root tab #11981

srirambv opened this issue Nov 15, 2017 · 11 comments
Assignees
@petemill
Labels
0.19.x issue first seen in 0.19.x bug feature/tabsbar priority/P4 Minor loss of function. Workaround usually present. QA/checked-Linux QA/checked-macOS QA/checked-Win64 QA/test-plan-specified regression release-notes/include
Milestone
0.22.x (Release C...

Comments

@srirambv
Copy link
Collaborator

srirambv commented Nov 15, 2017 •
edited by bsclifton
Loading

Test plan

See #12193

Description

Closing tab switches to the second tab from root tab

Steps to Reproduce

  1. Open 3 tab pages
  2. Close a tab
  3. Second tab from closed tab is set to focus instead of the immediate tab next to the closed one

Actual result:
11632

Expected result:
Should set focus to the immediate tab

Reproduces how often:
100%

Brave Version

about:brave info:

Brave 0.19.93
rev d2708a0
Muon 4.5.16
libchromiumcontent 62.0.3202.94
V8 6.2.414.42
Node.js 7.9.0
Update Channel Release
OS Platform Microsoft Windows
OS Release 10.0.15063
OS Architecture x64

Reproducible on current live release:

Additional Information

#11632 (comment)
cc: @cezaraugusto
@srirambv srirambv added 0.19.x issue first seen in 0.19.x feature/tabsbar regression labels Nov 15, 2017
@srirambv srirambv added this to the Triage Backlog milestone Nov 15, 2017
@LaurenWags
Copy link
Member

Seeing image below on the same build when closing (non-active) tab page, per discussion with @srirambv thought maybe it was related to this issue.
screen shot 2017-11-15 at 4 02 15 pm

@cezaraugusto
Copy link
Contributor

@LaurenWags changes added to fix this issue only created another check to ensure the preview is canceled, no changing for tab closing logic. Could you please open a new issue for that? Feel free to assign me too

@LaurenWags
Copy link
Member

Done @cezaraugusto :) logged #11985

@luixxiul luixxiul added the bug label Nov 16, 2017
@cezaraugusto cezaraugusto added the priority/P4 Minor loss of function. Workaround usually present. label Nov 21, 2017
@bsclifton bsclifton modified the milestones: Triage Backlog, Prioritized Backlog Nov 21, 2017
@srirambv
Copy link
Collaborator Author

srirambv commented Dec 5, 2017

This happens on the C63 build of 0.19.106
11981

@bsclifton bsclifton self-assigned this Dec 5, 2017
@bsclifton bsclifton modified the milestones: Backlog (Prioritized), 0.19.x Hotfix 7 (Release channel) Dec 5, 2017
@petemill petemill assigned petemill and unassigned bsclifton Dec 5, 2017
@petemill
Copy link
Member

petemill commented Dec 5, 2017

@srirambv I'm unable to see any regression here for the original description of this issue - from what I can tell the code is doing what it should - selecting the parent tab of the closed tab, which is the default setting:
image

Did you open all these tabs by clicking links from an original window? Perhaps you could give more detailed steps that I could reproduce - most importantly how you are opening the 3 tab pages of tabs, and which tab you are closing? Thanks

@srirambv
Copy link
Collaborator Author

srirambv commented Dec 5, 2017

@petemill Opened tabs by pressing and holding down the Ctrl+T to create tab pages. On 0.19.106 used Ctlr+Click to open tabs from the new tab tiles

@petemill
Copy link
Member

petemill commented Dec 5, 2017

thanks @srirambv I see

@petemill
Copy link
Member

petemill commented Dec 5, 2017

Confirmed reproducible on master + Windows and not macOS...

petemill added a commit that referenced this issue Dec 6, 2017
@petemill
Set next active tab before closing, instead of after. Avoids race-con…
0889ecc 
…dition with muon's index-change and active-change tab update events.

Fix #11981
@petemill petemill mentioned this issue Dec 6, 2017
Merged
8 tasks
petemill added a commit that referenced this issue Dec 7, 2017
@petemill
After tab close, set next active tab immediately, and only if differe…
d2db4c6 
…nt from the tab muon will set.

Avoids race condition that can occur after will-destory, before appState has been fully updated with any tab details that change from muon as a result of a tab closing.

Fix #11981
@NumDeP
Copy link

NumDeP commented Dec 7, 2017 •
edited
Loading

Hi @petemill I made mention of this here #11526

Perhaps I titled it incorrectly, though I only did so for the feature issues in Tab Settings to be merged and fixed together because I thought they were linked. You may have amended it sooner. Sorry.

petemill added a commit that referenced this issue Dec 7, 2017
@petemill
After tab close, set next active tab immediately, and only if differe…
dd96de6 
…nt from the tab muon will set.

Avoids race condition that can occur after will-destory, before appState has been fully updated with any tab details that change from muon as a result of a tab closing.

Fix #11981
petemill added a commit that referenced this issue Dec 7, 2017
@petemill
After tab close, set next active tab immediately, and only if differe…
bf11d30 
…nt from the tab muon will set.

Avoids race condition that can occur after will-destroy, before appState has been fully updated with any tab details that change from muon as a result of a tab closing.

Fix #11981
petemill added a commit that referenced this issue Dec 7, 2017
@petemill
After tab close, set next active tab immediately, and only if differe…
0acef5a 
…nt from the tab muon will set.

Avoids race condition that can occur after will-destroy, before appState has been fully updated with any tab details that change from muon as a result of a tab closing.

Fix #11981
Fix #11526
bsclifton pushed a commit that referenced this issue Dec 20, 2017
@petemill @bsclifton
After tab close, set next active tab immediately, and only if differe…
30dc2f1 
…nt from the tab muon will set.

Avoids race condition that can occur after will-destroy, before appState has been fully updated with any tab details that change from muon as a result of a tab closing.

Fix #11981
Fix #11526
@bsclifton bsclifton modified the milestones: 0.19.x Hotfix 9, 0.21.x (Developer Channel) Dec 20, 2017
petemill added a commit that referenced this issue Dec 22, 2017
@petemill
After tab close, set next active tab immediately, and only if differe…
b714140 
…nt from the tab muon will set.

Avoids race condition that can occur after will-destroy, before appState has been fully updated with any tab details that change from muon as a result of a tab closing.

Fix #11981
Fix #11526
This was referenced Feb 20, 2018
Closed
Closed
Closed
@btlechowski
Copy link
Contributor

Verified 0.21.9 Win64

This was referenced Mar 22, 2018
Closed
Closed
Closed
@srirambv
Copy link
Collaborator Author

srirambv commented Mar 22, 2018 •
edited by btlechowski
Loading

Verified on Windows x64

  • 0.22.6 e6ff4ea
  • libchromiumcontent: 65.0.3325.162
  • muon: 5.1.0

Verified on macOS 10.12.6 x64 using the following build:

  • 0.22.6 e6ff4ea
  • libchromiumcontent: 65.0.3325.162
  • muon: 5.1.0

Verified on Ubuntu 10.10 x64

  • 0.22.7 8bb7e77
  • libchromiumcontent: 65.0.3325.181
  • muon: 5.1.1

@kjozwiak kjozwiak mentioned this issue Mar 29, 2018
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@petemill petemill

Labels
0.19.x issue first seen in 0.19.x bug feature/tabsbar priority/P4 Minor loss of function. Workaround usually present. QA/checked-Linux QA/checked-macOS QA/checked-Win64 QA/test-plan-specified regression release-notes/include
Projects
None yet
Milestone
0.22.x (Release Channel)
Development

No branches or pull requests
8 participants
@petemill @luixxiul @cezaraugusto @bsclifton @srirambv @LaurenWags @NumDeP @btlechowski