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

Back/Forward bindings change wrong tab; causes empty new tab #8640

Closed
bsclifton opened this issue May 2, 2017 · 8 comments
Closed

Back/Forward bindings change wrong tab; causes empty new tab #8640

bsclifton opened this issue May 2, 2017 · 8 comments

Comments

@bsclifton
Copy link
Member

bsclifton commented May 2, 2017

Test plan

  1. Open wikipedia.org in one tab
  2. Open brave.com in a second tab
  3. While on brave.com, move your mouse over the wikipedia.org tab quickly and then push the back button
  4. Notice that wikipedia does not went back/forward

  • Did you search for similar issues before submitting this one?
    yes

  • Describe the issue you encountered:
    When using the browser, it's possible to get in a state where the back/forward buttons control the wrong tab (easy to spot because the title changes for the wrong one). When you visit the tab that was modified, it is now a dead tab.

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

  • Brave Version (revision SHA):
    0.15.2 Preview 2

  • Steps to reproduce:

    1. Open wikipedia.org in one tab
    2. Open brave.com in a second tab
    3. While on brave.com, move your mouse over the wikipedia.org tab quickly and then push the back button
    4. Notice that wikipedia went back/forward and not Brave. While you may have tab previewed, you didn't actually switch tabs.
    5. Click to load the wikipedia tab and notice it's a dead tab. Only seems to "die" when at the about:newtab page.
  • Actual result:
    Wrong page is navigated, tab is dead

  • Expected result:
    active page should be navigated

  • Will the steps above reproduce in a fresh profile? If not what other info can be added?

  • Is this an issue in the currently released version?

  • Can this issue be consistently reproduced?

  • Extra QA steps:
    1.
    2.
    3.

  • Screenshot if needed:

  • Any related issues:

@NejcZdovc
Copy link
Contributor

@bsclifton is this one the same as #8628?

@NejcZdovc
Copy link
Contributor

probably caused by #8336

@bsclifton bsclifton changed the title Back/Forward bindings change wrong tab; causes dead tab Back/Forward bindings change wrong tab; causes empty new tab May 2, 2017
@bsclifton
Copy link
Member Author

@NejcZdovc thanks for closing the other issue- I didn't notice it 😄 Good eye, @darkdh! (beat me to it!)

@NejcZdovc
Copy link
Contributor

Ok, so the problem is not in #8336, but in activeTab that is passed from main.js into navigator.js. We are getting activeTab here https://github.com/brave/browser-laptop/blob/master/js/components/main.js#L708.

In gif what I am doing is just printing out console.log("Active", activeTab && activeTab.get('title')) in main.js renderer. As you can see when you hover over tab active tab changes, but when you go back to first tab, active tab is not changed. It's only changed when you click on a tab.

13

@NejcZdovc
Copy link
Contributor

Ok I think that I found the main problem. I just pulled down version 0.14.1 and I noticed that this line https://github.com/brave/browser-laptop/blob/master/app/browser/tabs.js#L322 is not called when hovering over tab. Problem is that in 0.15.3 (latest master) this line is called. And with this line we change active tab value. Problem is that when you hover out of the preview tab, this line is not called so tab is not changed back. Will investigate further.

cc @bbondy @bsclifton @bridiver

@NejcZdovc
Copy link
Contributor

With version 0.15.0 this was changed and this line is triggered but it's working correctly. Investigating further. I can confirm that in 0.15.0 we don't have this problem.

@NejcZdovc
Copy link
Contributor

After some more hours I notice that production is working, but development was not working way before the release. I just tested browser on this commit and it's already failing there 0f54524.

Can it be related to node version? I am testing all of this with node 7.9.0. We switched to 7.9.0 lately.

@NejcZdovc
Copy link
Contributor

This was fixed with #8782.

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