667314 - Browser hang when attempting to close tab from ctrl-tab preview pane

Status: VERIFIED FIXED

(Firefox :: Tabbed Browser, defect)

Description

[Mark Straver]

User-Agent:       Mozilla/5.0 (Windows NT 5.1; rv:5.0) Gecko/20100101 Firefox/5.0
Build Identifier: Mozilla/5.0 (Windows NT 5.1; rv:5.0) Gecko/20100101 Firefox/5.0

The browser will hang when trying to close the current tab from the browser preview pane (ctrl-tab previews) if you have cycled through the the options in the pane completely. 

Reproducible: Always

Steps to Reproduce:
1. Set browser.ctrlTab.previews to true
2. Open more than two tabs
3. Press ctrl-tab repeatedly to cycle through the previews and the "view all x tabs" options until you are back at your first tab
4. Keep the pane open (don't release Ctrl) and attempt to close the tab with "w"

Actual Results:  
Browser hangs

Expected Results:  
Tab closes and browser updates the pane accordingly

According to an acquaintance, this also occurred in 4.x, and may be present in the current 6.x/7.x development versions as well.

A "non-responsive script" warning may sometimes also be given, in which case stopping the script will recover the browser's operation. If this doesn't happen, however, the only way to solve it is killing the app.
This script error points to browser.js @ line 3929

Comment 1

[(mostly gone) XtC4UaLL [:xtc4uall]]

Confirmed this against Mozilla/5.0 (Windows NT 5.1; rv:7.0a1) Gecko/20110626 Firefox/7.0a1 ID:20110626030756.

Comment 2

[(mostly gone) XtC4UaLL [:xtc4uall]]

Last good nightly: 2010-08-11 First bad nightly: 2010-08-12

Pushlog: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=ba956b17d834&tochange=cdfff833edf9

=> Bug 585361/Bug 582116?

The Script halts in tabbrowser.xml for me, btw.

Comment 3

[Alice0775 White]

Confirmed on
http://hg.mozilla.org/mozilla-central/rev/fc7d76664c79
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:7.0a1) Gecko/20110626 Firefox/7.0a1 ID:20110626030756

A script on this page may be busy, or it may have stopped responding. You can stop the script now, or you can continue to see if the script will complete.
Script: chrome://browser/content/browser.js:3898

 get tabList () {

 *snip*

     // Rotate the list until the selected tab is first
>    while (!list[0].selected)
       list.push(list.shift());

Regression window:
Works:
http://hg.mozilla.org/mozilla-central/rev/139e50f2d7a1
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b4pre) Gecko/20100812 Minefield/4.0b4pre ID:20100812025518
Fails:
http://hg.mozilla.org/mozilla-central/rev/cdfff833edf9
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b4pre) Gecko/20100812 Minefield/4.0b4pre ID:20100812040939
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=139e50f2d7a1&tochange=cdfff833edf9
Triggered by:
457ffad14bbd	Edward Lee — Bug 582116 - Provide a way to show certain tabs and get visible tabs [r=dao a=beltzner] Add showOnlyTheseTabs and visibleTabs to tabbrowser and update various uses such as tab selection. Test that tabs get hidden/shown when using this API and tab selection only deal with visible tabs while making sure there's always a visible tab.

Comment 4

[Notlost]

I'll confirm on windows 7. IT also leaves the dialog in front of everything and not responding. killing the script closes the open tab

Comment 5

[Tim]

Confirmed on XP SP3, Nightly

Comment 6

[Dão Gottwald [:dao]]

Attachment: patch

Comment 7

[Dão Gottwald [:dao]]

Attachment: patch

list.filter(... instead of Array.filter(list, ...

Comment 8

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 543898 [details] [diff] [review]
patch

Why does Array.filter(nodeList) vs. someArray.filter() make a difference? Why does leaving the "closing" tabs in for the rotation make a difference? Why does this bug occur in the first place? Maybe these things seem obvious to you once you've written the patch, but they're not obvious to me, so it'd help if you elaborated a little when requesting review :)

Comment 9

[:Gavin Sharp [email: [e-mail]]]

(In reply to comment #8)
> Why does leaving the "closing" tabs in for the rotation make a difference?

Oh, because the selected tab is closing (which explains the infinite loop) - OK.

The _closing stuff was already wrong, because _closing could only be a .closing tab, and visibleTabs would therefore never contain it, right?

Comment 10

[:Gavin Sharp [email: [e-mail]]]

Ah, yeah, broken by 457ffad14bbd looks like.

Comment 11

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 543898 [details] [diff] [review]
patch

Worth a comment explaining why .visibleTabs isn't suitable, and why you do the filtering in two steps ("selected tab can be closing").

Comment 12

[Dão Gottwald [:dao]]

http://hg.mozilla.org/mozilla-central/rev/5eb553dd2cea

Comment 13

[Vlad [QA]]

Setting resolution to Verified Fixed on Mozilla/5.0 (Windows NT 6.1; rv:7.0a2) Gecko/20110707 Firefox/7.0a2