Last Comment Bug 655797 - Recalculate whether to show close buttons after resizing tabs
: Recalculate whether to show close buttons after resizing tabs
Status: VERIFIED FIXED
:
Product: Firefox
Classification: Client Software
Component: Tabbed Browser (show other bugs)
: Trunk
: All All
: -- normal (vote)
: Firefox 6
Assigned To: Frank Yan (:fryn)
:
Mentors:
Depends on: 657463
Blocks: 465086
  Show dependency tree / graph
 
Reported: 2011-05-09 12:38 PDT by ithinc
Modified: 2011-07-01 01:04 PDT (History)
8 users (show)
hskupin: in‑testsuite?
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Attachments
patch (1.25 KB, patch)
2011-05-09 23:44 PDT, Frank Yan (:fryn) 		dao+bmo: review+
Details | Diff | Splinter Review
View All Add an attachment (proposed patch, testcase, etc.)

Description ithinc 2011-05-09 12:38:47 PDT 
User-Agent:       Mozilla/5.0 (Windows NT 5.1; rv:2.0.1) Gecko/20100101 Firefox/4.0.1
Build Identifier: Mozilla/5.0 (Windows NT 5.1; rv:2.0.1) Gecko/20100101 Firefox/4.0.1

Tab close buttons don't show after repeatedly closing some tabs and moving mouse out of the tab bar even if tabClipWidth is exceeded.

Reproducible: Always

Steps to Reproduce:
1. Open enough tabs to ensure tab close buttons are hidden
2. Close some tabs from the middle
3. Move mouse out of the tab bar


Actual Results:  
Tab close buttons don't show

Expected Results:  
Tab close button should show if tab width exceeds tabClipWidth
Comment 1 Frank Yan (:fryn) 2011-05-09 23:44:01 PDT 
Created attachment 531253 [details] [diff] [review]
patch
Comment 2 ithinc 2011-05-13 05:32:29 PDT 
Comment on attachment 531253 [details] [diff] [review]
patch

Review of attachment 531253 [details] [diff] [review]:
-----------------------------------------------------------------

So you're assuming the tab animation is always on. What if the tab animation is off?
Comment 3 Frank Yan (:fryn) 2011-05-13 11:36:06 PDT 
(In reply to comment #2)
> So you're assuming the tab animation is always on. What if the tab animation
> is off?

That'll be handled in bug 649671.
Comment 4 Daniel Holbert [:dholbert] 2011-05-13 13:34:26 PDT 
http://hg.mozilla.org/projects/cedar/rev/dbeeaffd5bea
Comment 5 ithinc 2011-05-13 17:25:12 PDT 
As the function name is _handleNewTab, I still think there's some improperness here.
Comment 6 Frank Yan (:fryn) 2011-05-13 23:06:24 PDT 
https://hg.mozilla.org/mozilla-central/rev/dbeeaffd5bea
Comment 7 :Gavin Sharp [email: gavin@gavinsharp.com] 2011-05-16 13:12:29 PDT 
(In reply to comment #5)
> As the function name is _handleNewTab, I still think there's some
> improperness here.

Yeah, this is gross. We should rename the function, and the very least.
Comment 8 Frank Yan (:fryn) 2011-05-16 14:00:54 PDT 
(In reply to comment #7)
> (In reply to comment #5)
> > As the function name is _handleNewTab, I still think there's some
> > improperness here.
> 
> Yeah, this is gross. We should rename the function, and the very least.

I don't think renaming the function to sth like _handleResizeTab is a good idea, since that function is called for new tabs that don't use the opening animation too.

I'll fix this in the just-filed bug 657463.
Comment 9 Henrik Skupin (:whimboo) 2011-05-19 05:27:15 PDT 
Can we get an automated test for fixes like that? A lot of code goes into the tabbrowser component but mostly we don't have tests. :/
Comment 10 Frank Yan (:fryn) 2011-05-23 14:40:53 PDT 
(In reply to comment #9)
> Can we get an automated test for fixes like that? A lot of code goes into
> the tabbrowser component but mostly we don't have tests. :/

I'd make a test for this in the patch for bug 657463 or bug 649671.
Comment 11 Vlad [QA] 2011-06-21 06:57:17 PDT 
If i delete some tabs from the middle, the "x" button still doesn't show. It shows only when a few tabs remains. This is the intended behavior?
Comment 12 George Carstoiu 2011-07-01 01:04:25 PDT 
Mozilla/5.0 (X11; Linux i686; rv:7.0a1) Gecko/20110630 Firefox/7.0a1

Verified issue on Ubuntu 11.04, Mac OS X 10.6, WinXP, Win 7 using steps from Comment 1.

Problem no longer reproducible -> setting status to Verified Fixed.
Note You need to log in before you can comment on or make changes to this bug.