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

Phantom Tab appears when middle clicking on a link with Switch to new tabs immediately #11070

Closed
LaurenWags opened this issue Sep 21, 2017 · 6 comments · Fixed by #11192
Closed

Phantom Tab appears when middle clicking on a link with Switch to new tabs immediately #11070

LaurenWags opened this issue Sep 21, 2017 · 6 comments · Fixed by #11192
Assignees
@darkdh
Labels
bug 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/exclude
Milestone
0.19.x (Beta Chan...

Comments

@LaurenWags
Copy link
Member

LaurenWags commented Sep 21, 2017 •
edited by luixxiul
Loading

Test Plan

See below

Description

Sometimes when clicking a URL link in another application (like Slack) a phantom tab appears.

Steps to Reproduce

  1. Go to about:preferences#tabs
  2. Turn on Switch to new tabs immediately
  3. Go to any sites in new tab
  4. Middle click on any links on the page
  5. Go back to the tab in step 3
  6. Middle click on any links on the page

Actual result:
kapture 2017-09-21 at 9 01 39

Expected result:
No phantom tab.

Reproduces how often:
Unsure. I have not been able to repro yet but this was reported by @darkdh

Brave Version

about:brave info:
Brave | 0.19.12
rev | 063f110
Muon | 4.4.19

Reproducible on current live release:
no. cannot be reproduced in 0.18.36.

Additional Information

See #11070 (comment)
Issue seems as though it was introduced with 0.19.7 because it cannot be reproduced with 0.19.6.
@LaurenWags LaurenWags added the bug label Sep 21, 2017
@darkdh
Copy link
Member

darkdh commented Sep 26, 2017 •
edited
Loading

I think it is because I have corrupted site entries contains only favicon like this
"https://medium.com/|0|0":{"favicon":"https://cdn-static-1.medium.com/_/fp/icons/favicon-rebrand-medium.3Y6xpZ-0FSdWDnPM3hSBIA.ico"}
"https://twitter.com/|0|0":{"favicon":"https://abs.twimg.com/favicons/favicon.ico"}
"https://feedly.com/|0|0":{"favicon":"https://feedly.com/favicon.ico"}
"https://www.facebook.com/|0|0":{"favicon":"https://www.facebook.com/rsrc.php/yl/r/H3nktOa7ZMg.ico"}

These have something in common, they are all pinned tabs and even if I remove them, they kept being generated on each relaunch

@darkdh
Copy link
Member

darkdh commented Sep 27, 2017

favicon only entry is addressed in #11161

@darkdh
Copy link
Member

darkdh commented Sep 27, 2017 •
edited
Loading

solid STR (reproducible on fresh profile):

  1. Go to about:preferences#tabs
  2. Turn on Switch to new tabs immediately
  3. Go to any sites in new tab
  4. Middle click on any links on the page
  5. Go back to the tab in step 3
  6. Middle click on any links on the page

@darkdh darkdh self-assigned this Sep 27, 2017
@darkdh darkdh changed the title Phantom Tab sometimes appears when clicking on a link in another application Phantom Tab appears when middle clicking on a link with about pages or pinned tab opened Sep 27, 2017
@bsclifton bsclifton added the priority/P4 Minor loss of function. Workaround usually present. label Sep 27, 2017
@bsclifton
Copy link
Member

Reproduced- this can be cause a crash if you right click the phantom tab (or drag it- anything which doesn't handle null properly). This causes you to force quit the app

@bsclifton bsclifton changed the title Phantom Tab appears when middle clicking on a link with about pages or pinned tab opened Phantom Tab appears when middle clicking on a link with Switch to new tabs immediately Sep 27, 2017
@bsclifton bsclifton mentioned this issue Sep 27, 2017
Closed
@LaurenWags
Copy link
Member Author

LaurenWags commented Sep 27, 2017 •
edited
Loading

I can repro in 0.19.7 but not 0.19.6.

0.19.6:
phantomtab0196

0.19.7:
phantomtab0197

@bsclifton bsclifton mentioned this issue Sep 27, 2017
Merged
11 tasks
@darkdh
Copy link
Member

darkdh commented Sep 27, 2017

This commit is the culprit
5d38cc1

@darkdh darkdh added this to the 0.19.x (Beta Channel) milestone Sep 28, 2017
darkdh added a commit that referenced this issue Sep 28, 2017
@darkdh
Using souceFrameIndex because new frame hasn't showed up in frames
509441b 
fix #11070

Auditors: @bbondy, @bsclifton

Test Plan: Covered by unittest
@darkdh darkdh mentioned this issue Sep 28, 2017
Merged
8 tasks
darkdh added a commit that referenced this issue Sep 28, 2017
@darkdh
Using souceFrameIndex because new frame hasn't showed up in frames
2d06add 
fix #11070

Auditors: @bbondy, @bsclifton

Test Plan: Covered by unittest
darkdh added a commit that referenced this issue Sep 28, 2017
@darkdh
Using souceFrameIndex because new frame hasn't showed up in frames
cbe93fb 
fix #11070

Auditors: @bbondy, @bsclifton

Test Plan: Covered by unittest
@bbondy bbondy mentioned this issue Oct 3, 2017
Closed
@srirambv srirambv mentioned this issue Oct 9, 2017
Closed
syuan100 pushed a commit to syuan100/browser-laptop that referenced this issue Nov 9, 2017
@darkdh @syuan100
Using souceFrameIndex because new frame hasn't showed up in frames
dbb9587 
fix brave#11070

Auditors: @bbondy, @bsclifton

Test Plan: Covered by unittest
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@darkdh darkdh

Labels
bug 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/exclude
Projects
None yet
Milestone
0.19.x (Beta Channel)
Development

Successfully merging a pull request may close this issue.

Using souceFrameIndex because new frame hasn't showed up in frames brave/browser-laptop
7 participants
@bbondy @kjozwiak @luixxiul @bsclifton @darkdh @srirambv @LaurenWags