351965 - Disable background close looks evil in the new themes

Status: RESOLVED FIXED

(Firefox :: Tabbed Browser, defect)

Description

[Phil Ringnalda (:philor)]

The visual refresh seems to have forgotten about browser.tabs.disableBackgroundClose - set it to true and your nonselected tabs have a strange and terrible and off-center strip of lowered opacity running clear through them. If nobody figures out a way to fix it so the buttons look disabled, rather than looking like someone took a swipe at them with a paintbrush, we should just go for no change in appearance, and if it's written to survive not having a default value, think about taking it out of firefox.js so people aren't tempted to see our brokenness.

Comment 1

[Phil Ringnalda (:philor)]

Attachment: Appearance on Linux

Comment 2

[Dão Gottwald [:dao]]

I don't understand how browser.tabs.disableBackgroundClose=true is different from browser.tabs.closeButtons=0. It looks different, sure, but to what end? imho browser.tabs.disableBackgroundClose can/should be removed.

Comment 3

[Phil Ringnalda (:philor)]

I'd say the primary difference is that you're several thousand times more likely to inadvertently double-click a background tab in the area where the close button isn't yet, and accidentally close it, with closeButtons=0.

Comment 4

[Phil Ringnalda (:philor)]

(In reply to comment #3)
> inadvertently double-click a background tab

For which Mano filed bug 352021

Comment 5

[Phil Ringnalda (:philor)]

Attachment: Remove it (trunk)

This should be a minimal removal - there's a little more related stuff in browser.css, but it's mingled with other stuff, and I'd rather take a new bug to strip it all out once the visual refresh lands on the trunk and the branch is gone. (Branch patch to come, because some unsynced whitespace at the end of a line on branch is confusing patch.)

Comment 6

[Phil Ringnalda (:philor)]

Attachment: Remove it (branch)

Comment 7

[Mike Connor [:mconnor]]

we typically don't block on bugs with hidden prefs, and never this late.

would likely take the patch though.

Comment 8

[Phil Ringnalda (:philor)]

Attachment: Really remove it (branch)

Bah, last branch patch didn't actually remove the pref. I must have been too shocked by all that trailing whitespace.

Comment 9

[Peter Kasting]

*** Bug 350034 has been marked as a duplicate of this bug. ***

Comment 10

[Asaf Romano (gone)]

Comment on attachment 237686 [details] [diff] [review]
Remove it (trunk)

You should also remove tab-close-button-placeholder entirely (i.e. xul.css, tabbrowser-tab binding in tabbrowser.xml and in globablBindings.xml of each theme, browser.css files of each toolkit theme).

Thanks for exposing browser.tabs.closeButtons while you're here.

Comment 11

[Phil Ringnalda (:philor)]

Attachment: Really remove it all (trunk)

Hmm, yeah. I don't know why removing something that's always display:none seemed like a scary change to me last night.

Comment 12

[Asaf Romano (gone)]

Comment on attachment 237921 [details] [diff] [review]
Really remove it all (trunk)

r=mano. On branch, you also need to tweak the winstripe-specific binding.

Comment 13

[Phil Ringnalda (:philor)]

Attachment: Really remove it all (branch)

philor:~/moz18/mozilla$ grep -r "button-place" ./
philor:~/moz18/mozilla$

Comment 14

[:Gavin Sharp [email: [e-mail]]]

mozilla/toolkit/themes/pinstripe/global/browser.css 	1.15
mozilla/toolkit/content/widgets/tabbrowser.xml 	1.200
mozilla/toolkit/content/xul.css 	1.86
mozilla/toolkit/themes/winstripe/global/browser.css 	1.19
mozilla/browser/app/profile/firefox.js 	1.161
mozilla/toolkit/themes/pinstripe/global/globalBindings.xml 	1.17

Comment 15

[Mike Connor [:mconnor]]

Comment on attachment 237938 [details] [diff] [review]
Really remove it all (branch)

a=mconnor on behalf of drivers, please get this in ASAP (tonight if possible)

Comment 16

[Asaf Romano (gone)]

1.8 branch:
mozilla/browser/app/profile/firefox.js 1.71.2.73
mozilla/toolkit/content/xul.css 1.61.2.20
mozilla/toolkit/content/widgets/tabbrowser.xml 1.103.2.92
mozilla/toolkit/themes/pinstripe/global/browser.css 1.7.4.19
mozilla/toolkit/themes/pinstripe/global/globalBindings.xml  1.3.12.13
mozilla/toolkit/themes/winstripe/global/browser.css  1.9.4.30
mozilla/toolkit/themes/winstripe/global/globalBindings.xml 1.3.24.4