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

VERIFIED FIXED in Firefox 7

Status

()

Firefox
Tabbed Browser
--
critical
VERIFIED FIXED
7 years ago
7 years ago

People

(Reporter: Mark Straver, Assigned: dao)

Tracking

({hang, regression})

Trunk
Firefox 7
hang, regression
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

7 years ago
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
Confirmed this against Mozilla/5.0 (Windows NT 5.1; rv:7.0a1) Gecko/20110626 Firefox/7.0a1 ID:20110626030756.
Keywords: hang
Version: unspecified → Trunk
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.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: regression

Comment 3

7 years ago
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.
(Assignee)

Updated

7 years ago
Blocks: 582116

Comment 4

7 years ago
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

7 years ago
Confirmed on XP SP3, Nightly
(Assignee)

Comment 6

7 years ago
Created attachment 543897 [details] [diff] [review]
patch
Assignee: nobody → dao
Status: NEW → ASSIGNED
Attachment #543897 - Flags: review?(gavin.sharp)
(Assignee)

Comment 7

7 years ago
Created attachment 543898 [details] [diff] [review]
patch

list.filter(... instead of Array.filter(list, ...
Attachment #543897 - Attachment is obsolete: true
Attachment #543898 - Flags: review?(gavin.sharp)
Attachment #543897 - Flags: review?(gavin.sharp)
(Assignee)

Updated

7 years ago
OS: Windows XP → All
Hardware: x86 → All
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 :)
(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?
Ah, yeah, broken by 457ffad14bbd looks like.
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").
Attachment #543898 - Flags: review?(gavin.sharp) → review+
(Assignee)

Comment 12

7 years ago
http://hg.mozilla.org/mozilla-central/rev/5eb553dd2cea
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → Firefox 7

Comment 13

7 years ago
Setting resolution to Verified Fixed on Mozilla/5.0 (Windows NT 6.1; rv:7.0a2) Gecko/20110707 Firefox/7.0a2
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.