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

Control+W does not work on tabs opened by clicking go back button #8974

Closed
luixxiul opened this issue May 20, 2017 · 8 comments
Closed

Control+W does not work on tabs opened by clicking go back button #8974

luixxiul opened this issue May 20, 2017 · 8 comments

Comments

@luixxiul
Copy link
Contributor

luixxiul commented May 20, 2017

Test plan

  1. Open about:preferences#tabs
  2. Change "When closing an active tab" setting to Select the last viewed tab
  3. Open a new tab
  4. Visit https://github.com on the tab
  5. Click 'Features' on the menu
  6. Click Go back button two times with the middle mouse button on the navigation bar
  7. Select the last tab
  8. Control + W
  9. Repeat ^

Focus should go form last tab to the parent tab (since it was last active).

Since the fix involved refactoring the logic for close tab, you may wish to test that all 3 of these options work as expected:
screen shot 2017-05-25 at 8 54 51 am

Description

Describe the issue you encountered: By changing the tab setting to Select the last closed tab, Control+W does not work on tabs opened by clicking go back button.

  • Platform (Win7, 8, 10? macOS? Linux distro?): tested on debian

  • Brave Version (revision SHA): 0.15.306

  • Steps to reproduce:

    1. Open about:preferences#tabs
    2. Change "When closing an active tab" setting to Select the last closed tab
    3. Open a new tab
    4. Visit https://github.com on the tab
    5. Click 'Features' on the menu
    6. Click Go back button two times with the middle mouse button on the nagivation bar
    7. Select the last tab
    8. Control + W
    9. Repeat ^
  • Actual result: only 1 tab can be closed

  • Expected result: both tabs should be closed

  • Extra QA steps:
    1.
    2.
    3.

  • Screenshot if needed:

  • Any related issues:

@bsclifton
Copy link
Member

When the tab is closed, we do calculate a new active frame (and this is correct). However, the tab state is never updated (ex: appState.get('tabs'). When any action happens which tries to use the TAB_ID_ACTIVE, it will then fail (because no tabs are marked as active).

Working on a fix...

bsclifton added a commit that referenced this issue May 23, 2017
I suspect the root cause is in Muon... specifically a `chrome-tabs-updated` event not firing
when tab.webContent.setActive(true) is called.

Auditors: @bridiver, @NejcZdovc

Test Plan:
See steps in #8974
@bsclifton
Copy link
Member

bsclifton commented May 23, 2017

Findings I had while making a work-around:

The underlying problem has these steps:

  • tab is cloned two times
  • focus is given to the 2nd cloned tab
  • ctrl + w happens
    • this triggers APP_TAB_CLOSE_REQUESTED
      • window is closed as expected
    • WINDOW_CLOSE_FRAME is fired + picked up by frameReducer
      • new active frame is picked
      • new frame state is committed to appState
      • APP_TAB_ACTIVATE_REQUESTED is fired
        • APP_FRAME_CHANGED fires (which includes tabState with active = false)

At this point, we're expecting APP_TAB_UPDATED
because it's changed above (tabs.setActive(action.get('tabId')))
..but it never happens... because it's already set as the active tab (APP_TAB_UPDATED not sent since value doesn't change)

bsclifton added a commit that referenced this issue May 23, 2017
@bridiver bridiver mentioned this issue May 24, 2017
7 tasks
bridiver pushed a commit that referenced this issue May 25, 2017
@bridiver
Copy link
Collaborator

the expected behavior in this issue is actually wrong. With a setting of Select the last viewed tab (there is no Select the last closed tab setting), it should go back to the original github page, not the second clone

@luixxiul
Copy link
Contributor Author

Select the last closed tab

I don't remember correctly if I misread something (sorry it was confusing), yet the point is you cannot close the 2 tabs consecutively.

@bridiver
Copy link
Collaborator

there are actually 2 bugs here. The first is (with that setting) it should return to the original github.com tab. The second is the issue with closing the second tab

@bsclifton
Copy link
Member

Fixed in dev-channel with #9023
Fixed in master with #9057

@luixxiul
Copy link
Contributor Author

The test plan works, still I found another issue (which should not be a blocker I think) by following the STR with one about page (about:preferences) opened. I'm going to log the issue as a follow-up.

@luixxiul
Copy link
Contributor Author

I logged here: #9065

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