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

Always switch back to active tab after sequentially closing tabs #9539

Merged
merged 2 commits into from
Jun 20, 2017

Conversation

cezaraugusto
Copy link
Contributor

@cezaraugusto cezaraugusto commented Jun 17, 2017

Auditors: @bridiver

Fix #9306

Automated Test Plan:

npm run test -- --grep="sequentially closing tabs"

Manual Test Plan:

test 1

  1. Open 4 tabs
  2. Set 4th as active
  3. Hover over 3rd, close it and don't move your mouse
  4. Close 4th (now 3rd) tab
  5. Active tab content should show

test 2

  1. Open 3 tabs
  2. Set 1st as active
  3. Hover over 2nd, close it
  4. Active tab should not be changed (should still be the 1st)

@cezaraugusto cezaraugusto added this to the 0.17.x (Beta Channel) milestone Jun 17, 2017
@cezaraugusto cezaraugusto self-assigned this Jun 17, 2017
@cezaraugusto cezaraugusto requested a review from bridiver June 17, 2017 19:05
@cezaraugusto cezaraugusto changed the title Add test case for sequentially closing tabs Always switch back to active tab after sequentially closing tabs Jun 18, 2017
@cezaraugusto
Copy link
Contributor Author

@bridiver issue was fixed but regressed on the same issue as per this PR comment, active tab is switching after close even if I don't close the active tab. Here are the steps to repro:

  1. Open four tabs
  2. Set first to active
  3. Hover over second, close it

Expected is to preview the third (now second) tab, given the first tab is active, but this change is switching active tab to next frame.

@bridiver
Copy link
Collaborator

@cezaraugusto fixed my dumb mistake

@bsclifton
Copy link
Member

Great job on this guys 😄

@cezaraugusto @bridiver the test supplied seems to always fail. Does this require a version of Muon > 4.0.5?

@bridiver
Copy link
Collaborator

passes for me

@bridiver
Copy link
Collaborator

I see a lot of other test failures in ci, but not sure if they are related or not

@bridiver
Copy link
Collaborator

also verified that it passes when cherry-picked into 0.17. I'm going to run the full test suite locally

@bridiver
Copy link
Collaborator

1) Bravery Panel Tracking Protection stats detects blocked elements in private tab:
     Error: timeout of 30000ms exceeded. Ensure the done() callback is being called in this test.
  

  2) Bravery Panel Adblock stats without iframe tests blocks websocket tracking:
     WaitUntilTimeoutError: element (#result) still not visible after 10000ms
      at elementIdDisplayed("0.0363547451694588-1") - isVisible.js:71:55

  3) Bravery Panel Adblock stats without iframe tests detects https upgrades in private tab:
     WaitUntilTimeoutError: Promise was rejected with the following reason: timeout
      at elementIdText("0.4433965589153763-4") - getText.js:35:50

  4) Bravery Panel Adblock stats without iframe tests blocks scripts in a private tab:
     WaitUntilTimeoutError: Promise was rejected with the following reason: timeout
      at elementIdText("0.20697618119568317-8") - getText.js:35:50

  5) Bravery Panel Adblock stats without iframe tests blocks fingerprinting:
     Uncaught Couldn't connect to selenium server
  Error

  6) Bravery Panel Adblock stats without iframe tests block device enumeration:
     WaitUntilTimeoutError: Promise was rejected with the following reason: Error: chrome not reachable
      at getText("[data-test-id="fpStat"]") - braveryPanelTest.js:788:23

  7) Bravery Panel Adblock stats without iframe tests "after each" hook for "block device enumeration":
     TypeError: this.app.client.waitForBrowserWindow is not a function
      at promises.push.callback (test/lib/brave.js:1052:23)
      at node_modules/async/internal/parallel.js:31:39
      at replenish (node_modules/async/internal/eachOfLimit.js:64:17)
      at node_modules/async/internal/eachOfLimit.js:68:9
      at eachOfLimit (node_modules/async/eachOfLimit.js:39:36)
      at node_modules/async/internal/doLimit.js:9:16
      at _parallel (node_modules/async/internal/parallel.js:30:5)
      at series (node_modules/async/series.js:83:26)
      at Promise (test/lib/brave.js:1061:7)
      at Context.stopApp (test/lib/brave.js:1060:12)
      at Context.<anonymous> (test/lib/brave.js:196:30)
      at Hook.Runnable.run (node_modules/co-mocha/lib/co-mocha.js:43:16)

  8) Bravery Panel Adblock stats without iframe tests "before each" hook for "detects blocked elements in iframe in private tab":
     TypeError: Cannot read property 'call' of undefined
      at Hook.Runnable.run (node_modules/co-mocha/lib/co-mocha.js:43:16)
      at process._tickCallback (internal/process/next_tick.js:109:7)

  9) Bravery Panel Adblock stats iframe tests "after each" hook for "detects blocked elements in iframe in private tab":
     TypeError: this.app.client.waitForBrowserWindow is not a function
      at promises.push.callback (test/lib/brave.js:1052:23)
      at node_modules/async/internal/parallel.js:31:39
      at replenish (node_modules/async/internal/eachOfLimit.js:64:17)
      at node_modules/async/internal/eachOfLimit.js:68:9
      at eachOfLimit (node_modules/async/eachOfLimit.js:39:36)
      at node_modules/async/internal/doLimit.js:9:16
      at _parallel (node_modules/async/internal/parallel.js:30:5)
      at series (node_modules/async/series.js:83:26)
      at Promise (test/lib/brave.js:1061:7)
      at Context.stopApp (test/lib/brave.js:1060:12)
      at Context.<anonymous> (test/lib/brave.js:196:30)
      at Hook.Runnable.run (node_modules/co-mocha/lib/co-mocha.js:43:16)

@bridiver
Copy link
Collaborator

most appear to be intermittent and not related to this PR

@cezaraugusto
Copy link
Contributor Author

cezaraugusto commented Jun 20, 2017

works fine to me, also when rebasing against master/0.17.x

cezaraugusto and others added 2 commits June 20, 2017 15:32
Auditors: @bridiver
Test Plan:
npm run test -- --grep="sequentially closing tabs"
@bbondy bbondy merged commit 81f05a4 into master Jun 20, 2017
bbondy added a commit that referenced this pull request Jun 20, 2017
Always switch back to active tab after sequentially closing tabs
bbondy added a commit that referenced this pull request Jun 20, 2017
Always switch back to active tab after sequentially closing tabs
@cezaraugusto cezaraugusto deleted the test/9306 branch June 20, 2017 19:22
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.

Closing Tabs Results in White Page
4 participants