483140 - Make the pop-up blocker's text field more visible in the nib

Status: RESOLVED FIXED

(Camino Graveyard :: Annoyance Blocking, defect)

Description

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

In bug 483033 we just saw a reoccurrence of an old problem (we saw it in 1.5 as well) where the text field in the pop-up blocker overlaps the leftmost button, often significantly.  When I was attempting to figure out what was wrong, it took me quite a bit of time being stumped, and only after I finally ran the interface simulator and noticed some "fuzzy" stuff on top of the button did I realize the problem was the overlap.

Unfortunately, this is extremely easy to miss because the text field uses grey text (looks great on our black gradient) and the color of the nib background is also grey.

Is there some way that we can either 

1) change the color of the text in the nib to provide contrast with the background, but then set the grey color we desire from code at runtime, or
2) add some sort of dark background to the nib that's ignored/drawn over by our gradient-drawing code?

If there's nothing we can do, so be it, but if there is a way, doing so would make it much easier for l10n teams to notice this problem and keep us from shipping it.

Comment 1

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

cl said the other day that doing 1) should be fairly easy (or at least possible), so I'm going to confirm this.  Won't block on this, but if someone has a few cycles to implement it, I'd be grateful.

Comment 2

[Christopher Henderson]

Attachment: Fix v1.0

Programmatically sets the text colour of the popup blocked label to 90% gray, the same as was in the nib.

Comment 3

[Christopher Henderson]

Attachment: New PopupBlockView.nib

New PopupBlockView nib, with the label text colour changed to pure black. No other changes made. I can resave the nib in 10.4 this weekend, which will probably be before the patch passes sr.

Comment 4

[Stuart Morgan]

Comment on attachment 376387 [details] [diff] [review]
Fix v1.0

Preemptive sr=smorgan on code. Someone should still do a sanity test though.

Do we care about the 10.5-itude of this nib? I'm inclined to think not, since it's loaded on demand.

Comment 5

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Comment on attachment 376387 [details] [diff] [review]
Fix v1.0

Nib+patch passes sanity-check.

Since this isn't in a critical perf path on the tinderbox or in real usage (e.g., prefs), I'll go ahead and land this one.

Comment 6

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Checked in on cvs trunk.  Thanks for this fix, hendy!