438302 - Remove Pinstripe text-shadow hacks

Status: VERIFIED FIXED

(Firefox :: Theme, defect)

Description

[Dão Gottwald [:dao]]

Once bug 10713 is fixed, we should kill the XBL hack that currently creates the shadow effect in the Mac theme.

Comment 1

[[e-mail]]

No you can't. See bug 10713 comment #159.

Comment 2

[Dão Gottwald [:dao]]

great

Comment 3

[Dave Townsend [:mossop] (PTO until July 20)]

Attachment: possible patch

Dammit, I really should have searched for this before I spent some time trying to actually do this. Anyway here is a patch that I think is about right assuming bug 438517 is ever fixed.

Comment 4

[Dão Gottwald [:dao]]

Attachment: patch

Comment 5

[Dão Gottwald [:dao]]

Attachment: patch v2

actually, I don't think we want to add the shadow to all toolbar buttons...

Comment 6

[Dão Gottwald [:dao]]

Attachment: patch v2

tabbox.css was missing

Comment 7

[Markus Stange [:mstange]]

(In reply to comment #5)
> Created an attachment (id=341248) [details]
> patch v2
> 
> actually, I don't think we want to add the shadow to all toolbar buttons...

Why not? That's what I did when I worked on it this morning... :)

Comment 8

[Markus Stange [:mstange]]

And I think text-shadow: 0 1px rgba(255, 255, 255, 0.4); would be more accurate, even though it usually doesn't make a difference.

Comment 9

[Dão Gottwald [:dao]]

(In reply to comment #7)
> (In reply to comment #5)
> > Created an attachment (id=341248) [details] [details]
> > patch v2
> > 
> > actually, I don't think we want to add the shadow to all toolbar buttons...
> 
> Why not?

Are other OS X apps doing that?

(In reply to comment #8)
> And I think text-shadow: 0 1px rgba(255, 255, 255, 0.4); would be more
> accurate, even though it usually doesn't make a difference.

Dunno, rgb(240, 240, 240) at 50% opacity is what pinstripe uses currently.

Comment 10

[Markus Stange [:mstange]]

(In reply to comment #9)
> (In reply to comment #7)
> > (In reply to comment #5)
> > > Created an attachment (id=341248) [details] [details] [details]
> > > patch v2
> > > 
> > > actually, I don't think we want to add the shadow to all toolbar buttons...
> > 
> > Why not?
> 
> Are other OS X apps doing that?

Yes. On OS X, all text that's directly on the dark grey chrome background has a shadow. And since we're styling toolbars with that color by default, I think we should make the text-shadow the default, too.

Comment 11

[Dão Gottwald [:dao]]

Attachment: patch v3

Now again for all toolbar buttons and with rgba(255, 255, 255, 0.4) (although I don't know why that's supposed to be better.)
Apparently we can also remove pinstripe's tabbrowser-tab binding, as Mossop already did it correctly in his first patch.

Comment 12

[Ryan Flint [:rflint] (ping via IRC for reviews)]

Comment on attachment 341273 [details] [diff] [review]
patch v3

Please wait until after the current txul regression is sorted out if you're planning on landing this for beta 1.

Comment 13

[Dão Gottwald [:dao]]

http://hg.mozilla.org/mozilla-central/rev/ffd6a43042b9

Comment 14

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Verified by check-in on trunk.