Remove Pinstripe text-shadow hacks

VERIFIED FIXED in Firefox 3.1b2

Status

()

Firefox
Theme
VERIFIED FIXED
10 years ago
10 years ago

People

(Reporter: dao, Assigned: dao)

Tracking

Trunk
Firefox 3.1b2
All
Mac OS X
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 4 obsolete attachments)

(Assignee)

Description

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

Comment 1

10 years ago
No you can't. See bug 10713 comment #159.
(Assignee)

Comment 2

10 years ago
great
Depends on: 438517
No longer depends on: 10713
(Assignee)

Updated

10 years ago
No longer blocks: 434348
Created attachment 334776 [details] [diff] [review]
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.
(Assignee)

Updated

10 years ago
Assignee: nobody → dao
(Assignee)

Updated

10 years ago
Summary: Use CSS text-shadow for tabbrowser tabs on OS X → Remove Pinstripe text-shadow hacks
(Assignee)

Comment 4

10 years ago
Created attachment 341247 [details] [diff] [review]
patch
Attachment #334776 - Attachment is obsolete: true
Attachment #341247 - Flags: review?(rflint)
(Assignee)

Comment 5

10 years ago
Created attachment 341248 [details] [diff] [review]
patch v2

actually, I don't think we want to add the shadow to all toolbar buttons...
Attachment #341247 - Attachment is obsolete: true
Attachment #341248 - Flags: review?(rflint)
Attachment #341247 - Flags: review?(rflint)
(Assignee)

Comment 6

10 years ago
Created attachment 341253 [details] [diff] [review]
patch v2

tabbox.css was missing
Attachment #341248 - Attachment is obsolete: true
Attachment #341253 - Flags: review?(rflint)
Attachment #341248 - Flags: review?(rflint)
(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... :)
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.
(Assignee)

Comment 9

10 years ago
(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.
(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.
(Assignee)

Comment 11

10 years ago
Created attachment 341273 [details] [diff] [review]
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.
Attachment #341253 - Attachment is obsolete: true
Attachment #341273 - Flags: review?(rflint)
Attachment #341253 - Flags: review?(rflint)
(Assignee)

Updated

10 years ago
Blocks: 458111
Attachment #341273 - Flags: review?(rflint) → review+
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.
(Assignee)

Updated

10 years ago
Keywords: checkin-needed
(Assignee)

Comment 13

10 years ago
http://hg.mozilla.org/mozilla-central/rev/ffd6a43042b9
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3.1b2
Hardware: PC → All
(Assignee)

Updated

10 years ago
Depends on: 477693
Verified by check-in on trunk.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.