STR: 1. Open enough tabs with CTRL+T to trigger tab overflow, then several additional tabs. 2. Scroll to and select the rightmost tab 3. Close it (e.g. via CTRL+W or by middle-clicking) Actual results: The tab gets closed and the tab to the left gets selected *then* the newly selected tab gets scrolled to fill up the freed space. This looks really annoying and visually displeasing. Expected results: Scrolling the tabstrip and closing the current tab/selecting the tab to the left should happen simultaneously, just like when you close tabs in the non-overflow case.

This happens in trunk builds as well as 1.8 builds. I'm tempted to ask for blocking fx2, but I won't given that tab overflow is considered an edge case and this is just a visual problem, albeit an annoying one.

I see the problem, and I have a change that appears to solve it, but it feels risky for FF2. in removeTab(), we currently remove the tab and the select the new tab. // Remove the tab this.mTabContainer.removeChild(oldTab); // invalidate cache, because mTabContainer is about to change this._browsers = null; this.mPanelContainer.removeChild(oldBrowser.parentNode); // Find the tab to select ... this.updateCurrentBrowser(); // see comment above destroy above oldBrowser.focusedWindow = null; oldBrowser.focusedElement = null; if we select the tab, and then remove it, the "looking bad on close" issue goes away. // Find the tab to select ... this.updateCurrentBrowser(); // Remove the tab this.mTabContainer.removeChild(oldTab); // invalidate cache, because mTabContainer is about to change this._browsers = null; this.mPanelContainer.removeChild(oldBrowser.parentNode); // see comment above destroy above oldBrowser.focusedWindow = null; oldBrowser.focusedElement = null; but, this sort of change makes me pretty nervous right before we ship, as I'm not aware of all the implications of changing the order like that.

clearing the TM. note, this is a new issue (for FF2) because of the scrolling tab strip.

I think we should first move (and clean up!) the scroll-to-end code from adjustTabstrip to removeTab so it isn't done post-reflow.

See also bug 324449, moving that code around can have other consequences.

Gavin, was this a reply to comment 2 or comment 4?

Sorry, yes, it was a reply to comment 2, and perhaps not that useful. I just recalled changing that code before and was trying to figure out where.

> I think we should first move (and clean up!) the scroll-to-end code from > adjustTabstrip to removeTab so it isn't done post-reflow. I like your idea of removing the aRemovingTab parameter from adjustTabstrip(). I'm trying out a patch to move the scroll-to-end code from adjustTabstrip() to removeTab() [after the point where we call removeChild()] asaf, what did you have in mind for cleaning up this code?

schrep, this is one of those UI nits that I mentioned. it's a "new" issue with FF2, as it started being an issue with tab overflow. I'd like to fix this, but it may not be bad enough to block ff2. ben/beltzner/mconnor, comments?

asaf wrote: 1) first of all this isn't a "hack" anymore :) 2) declare and set tabWidth inside the if block (that's, don't get it if we don't need it) 3) we don't need the tabstrip var i think 4) you may also want to put the whole thing in a try catch block I kept tabstrip (comment 3) to keep the code cleaner.

Comment on attachment 231842 [details] [diff] [review] updated patch, per asaf's review in irc r=mano.

fixed on the trunk. thanks again to asaf for suggesting the fix.

Comment on attachment 231842 [details] [diff] [review] updated patch, per asaf's review in irc a=beltzner for drivers

fixed on the branch