585393 - left tab title text shifts when a new tab is added

Status: RESOLVED FIXED

(Firefox :: Theme, defect)

Description

[Robert Sayre]

STR:

1.) open Minefield
2.) first tab is titled "Minefield Start Page"
3.) add and close a second tab
4.) observe the first tab's title text shift as the close button is added removed

Comment 2

[Johnathan Nightingale [:johnath]]

This looks like a similar problem to bug 579323. Roc, agree?

Comment 4

[Aaron Train [:aaronmt]]

This is marked as a dupe of bug 579323, but now that bug 579323 landed, I am still observing this behaviour in 20100831.

Comment 5

[Dão Gottwald [:dao]]

The tab label is centered on OS X, so when the tab's close button is removed, the label's available width increases and the center shifts to the right. This isn't a new problem as far as I can see.

Comment 6

[Johnathan Nightingale [:johnath]]

I agree with Dao, I don't think this is a regression, or bad enough to block. Roc, please feel free to re-nom if you disagree.

Comment 7

[Robert Sayre]

(In reply to comment #6)
> please feel free to re-nom if you disagree.

I do.

Comment 8

[Robert Sayre]

Here is a YouTube video showing the behavior at 50% speed:

http://www.youtube.com/watch?v=iA2rsmLprYg

The mistake is to remove the space needed for the tab close button at all. When the user opens a new tab, the browser distractingly realigns the text in the old tab. The special case of one tab open is not that interesting to optimize for, because the full title is always shown in the title bar in that case.

This behavior also causes related bugs, since the "New Tab" text is realigned just as the second tab reaches full width.

All tabs should appear exactly the same. These non-uniform alignment shifts give the software a low quality feel.

Comment 9

[Robert Sayre]

It's been three weeks since I renominated this bug.

Comment 10

[Mike Beltzner [:beltzner, not reading bugmail]]

It's been four weeks since I've been to the gym.

Rob's right, this is a visible twitch to the UI which results in an overall lesser feel. Over to Faaborg to determine where it falls in the pantheon of polish bugs, and he can renominate for blocking at that time.

Best solution, as Rob says (and Safari does) is to just continually reserve the space and save us the layout hassle. I don't think this blocks release, but agree that it's a high leverage low cost polish item that should be tracked in Alex's list.

Comment 11

[Frank Yan (:fryn)]

I'm pretty certain that this is fixable with just fancy CSS selector magic. I'll give it a shot.

Comment 12

[Frank Yan (:fryn)]

Attachment: patch

Simple patch.

It reserves the space on OS X but not on other platforms.
It also fixes (without any additional code) the same issue when browser.tabs.closeButtons is a value other than the default 1.

Comment 13

[Dão Gottwald [:dao]]

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

Rather than adding "noclosemac", I think you should handle this completely in pinstripe's browser.css. tabbrowser.css and tabbrowser.xml don't need to know about this. But: I'm not convinced that this is even the solution we want, as it reduces the space available for tab titles when the tabs are at their minimum size.

Comment 14

[Dão Gottwald [:dao]]

Bug 349108 would solve this too.

Comment 15

[Frank Yan (:fryn)]

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

(In reply to comment #13)

AFAIK, browser.css does not have access to the anonymous nodes. Even if it does, there is no way to know when a tab's nextSibling is being closed in CSS, so tab:only-of-type or tab:only-child is insufficient to fix this.

My patch only reduces the space available for the tab title when there is only 1 tab. Please read the patch. I should have clarified that the space is only reserved when only one tab is open.

Comment 16

[Frank Yan (:fryn)]

(In reply to comment #14)
> Bug 349108 would solve this too.

It's not clear that we still want that UI. I talked to Limi about that, and we came to the conclusion that close buttons on the right side make more sense.

Comment 17

[Dão Gottwald [:dao]]

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

(In reply to comment #15)
> AFAIK, browser.css does not have access to the anonymous nodes.

It does.

> there is no way to know when a tab's nextSibling is being closed in CSS,
> so tab:only-of-type or tab:only-child is insufficient to fix this.

Simple solution is to just not call adjustTabstrip in _beginRemoveTab as your patch does.

Comment 18

[Frank Yan (:fryn)]

Attachment: simpler patch

(In reply to comment #17)

When you have two tabs, and one closes, selection shifts immediately to the other tab, so the close button immediately appears on that tab. adjustTabStrip should called immediately in order to make the close button disappear, but if you find that brief state to be acceptable, this simpler patch will do.

It will reserve the space when browser.tabs.closeButtons is 2 or 3 though, but I figure that we don't need to bother with those obscure pref values.

Comment 19

[Dão Gottwald [:dao]]

(In reply to comment #18)
> When you have two tabs, and one closes, selection shifts immediately to the
> other tab, so the close button immediately appears on that tab.

It doesn't appear, it was already there in the case of two tabs, so what happens is that it disappears when the second tab has been closed.

> adjustTabStrip
> should called immediately in order to make the close button disappear, but if
> you find that brief state to be acceptable, this simpler patch will do.

It's only tangentially related to this bug, anyway.

> It will reserve the space when browser.tabs.closeButtons is 2 or 3 though, but
> I figure that we don't need to bother with those obscure pref values.

So why don't you use :only-of-type/:only-child? Alternative solution would be to not overload "noclose" the way it's currently overloaded in order to allow pinstripe to differentiate.

Comment 20

[Frank Yan (:fryn)]

(In reply to comment #19)
> So why don't you use :only-of-type/:only-child?

If I use :only-of-type/:only-child, when there is one tab and another is opened, the tab text will shift to the right until adjustTabStrip is called in _handleNewTab to change closebuttons to "alltabs". If you think un-overloading "noclose" is worth it, let it be known, and I'll write a patch with that. Otherwise, this should do.

Comment 21

[Dão Gottwald [:dao]]

browser.tabs.closeButtons=3 used to be the default, so a few people are probably attached to that. Should be simple enough, too.

Comment 22

[Frank Yan (:fryn)]

Attachment: patch v3

It turns out that we no longer use "closeatend", so I removed that from tabbrowser.css.

Comment 23

[Frank Yan (:fryn)]

Attachment: patch v4

fixed a selector typo.

Comment 24

[Dão Gottwald [:dao]]

Comment on attachment 499258 [details] [diff] [review]
patch v4

It's not really clear what "hidden" means as opposed to "noclose". How about "never" for 2 and 3?

Comment 25

[Frank Yan (:fryn)]

(In reply to comment #24)
> Comment on attachment 499258 [details] [diff] [review]
> patch v4
> 
> It's not really clear what "hidden" means as opposed to "noclose". How about
> "never" for 2 and 3?

That works. We could also removeAttribute("closebuttons")
Which?

Comment 26

[Dão Gottwald [:dao]]

[no attribute] vs. "noclose" or "hidden" also seems subtle.

Comment 27

[Frank Yan (:fryn)]

Attachment: patch v5

Comment 28

[Frank Yan (:fryn)]

Pushed yesterday. Forgot to list changeset.

https://hg.mozilla.org/mozilla-central/rev/11362bbb1bfa