froodian (Ian Leue) Reporter
	Description • 18 years ago

This can happen with a real use-case, I'm just using a very narrow window for illustration's purposes.

1. Open a Camino window, resize to 525 px
2. Open five tabs (max that will fit in a window that size)
3. With the rightmost tab focused, open a link in the background

What happens:  Currently focused tab is now off-screen
What should happen:  Implementation should keep the current tab visible

	Desmond Elliott
	Comment 1 • 18 years ago

Confirming bug, working on fix.

	Desmond Elliott
	Comment 2 • 18 years ago

Gives us selected tab is always visible for free which is quite a nice bonus.

The maths might be a little bit assumptive, but I think it covers all cases.

Spaced out and commented this part of the method to make it easier to read - it took me a few minutes to understand it... and I wrote it!

	Stuart Morgan Assignee
	Comment 3 • 18 years ago

Comment on attachment 248792 [details] [diff] [review]
Makes sure that overflow tabs doesn't displace the currently selected tab

> Gives us selected tab is always visible for free which is quite a nice bonus.

Except that preventing people from scrolling away from the selected tab defeats part of the purpose of scrolling tabs.

	Stuart Morgan Assignee
	Comment 4 • 18 years ago

The way this should probably be done is to check if the current tab is visible before recomputing the visible tab count, and then adjusting if it's not visible by the end.

	Alexander
	Comment 5 • 18 years ago

When tabs go offscreen it is also IMPOSSIBLE to close them without closing others. Please make scrolling arrows to navigate additional tabs.

	Stuart Morgan Assignee
	Comment 6 • 18 years ago

We have them on trunk; this bug is about a specific minor bug in that implementation.

	Smokey Ardisson (offline for a while; not following bugs - do not email)
	Comment 7 • 17 years ago

Setting priority per 1.6 roadmap.

	Stuart Morgan Assignee
	Comment 8 • 17 years ago

Sorry Ian, this annoyed me enough recently that I just fixed it. This also fixes the related problem of having the selected tab fall off the edge when shrinking a window.

	Sean Murphy
	Comment 9 • 17 years ago

(In reply to comment #8)
> Created an attachment (id=271682) [details]
> fix

I wonder if |layoutButtonsPreservingVisibility:| is a little misleading in that visibility can still be preserved even when supplying 'NO' as the argument.  This confused me upon first glance, as it would seem that explicitly specifying a certain value would dictate only that behavior.  I needed to scan through the method implementation to learn how it actually worked.  

What was unclear to me at first was that preserveVisibility = NO does not mean never maintain visibility, but rather 'maintain visibility if needed'.  YES, more predictably, forces visibility no matter what.

In defense, you did comment inside the method explaining |keepCurrentTabVisible| and it is private and therefore usually called with implementation knowledge in mind.

This method name's self-documenting aspect aside, the actual visibility preservation itself works perfectly in every single tab use case I performed.  I'll wait to hear your thoughts about what I mentioned above; If we decide that's not a problem, r=me.

	Stuart Morgan Assignee
	Comment 10 • 17 years ago

Comment on attachment 271682 [details] [diff] [review]
fix

I see your point, but I wasn't able to come up with a better name (and my earlier attempt to have that logic outside the layout method was ugly). I'm not too concerned, since I think "Yes, I care about preserving visibility" and "No, I don't care, so do whatever" are reasonable; I'll clarify the method-level comment if no-one has any suggestions for a clearer name.

	Mike Pinkerton (not reading bugmail)
	Comment 11 • 17 years ago

Comment on attachment 271682 [details] [diff] [review]
fix

sr=pink

	Stuart Morgan Assignee
	Comment 12 • 17 years ago

Landed on trunk and MOZILLA_1_8_BRANCH.