527248 - Incorrect bookmark bar colors when using a display profile with a gamma NOT set to 1.8

Status: RESOLVED FIXED

(Camino Graveyard :: Toolbars & Menus, defect)

Description

[R P Mozley]

User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X 10.4; en-GB; rv:1.9.0.15) Gecko/2009102701 Camino/2.0 (like Firefox/3.0.15)
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X 10.4; en-GB; rv:1.9.0.15) Gecko/2009102701 Camino/2.0 (like Firefox/3.0.15)

The bookmarks bar colours are changing dependent on what display profile the user is using. Anything other than a profile using a gamma of 1.8 will make the gradient colours different to what is set in the source code.

Reproducible: Always

Steps to Reproduce:
1. Set a display profile to one that doesn't use a gamma of 1.8 (Apple standard).
2. Open Camino 2.0.
3. Check values using Digital Color Meter (in the background).
Actual Results:  
Bookmark bar colours appear out of line with source code values.

Expected Results:  
Bookmark colours appear with correct values as set in source code.

Mac OS X 10.4.11 iBook G4

Comment 1

[R P Mozley]

Attachment: Possible patch.

We still need to use a device dependent profile to keep the gradient colours the same as set.

Comment 2

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

I'm confused, though; shouldn't this have been happening all the time on Intel, which always had a minimum that wasn't < 10.4?  Was the ifdef that was removed in bug 425519 always wrong?

I'm guessing this is the right thing to do, though, since the other color functions in CHGradient are using NSDeviceRGBColorSpace currently.

Comment 3

[Stuart Morgan]

According to the docs those calls should be equivalent. Are you actually seeing a difference with this patch?

Comment 4

[R P Mozley]

(In reply to comment #3)
> According to the docs those calls should be equivalent. Are you actually seeing
> a difference with this patch?

Yes I'm seeing a difference alright. It does indeed say in the documentation that in 10.4 it becomes generic RGB but in practice they don't appear to be equivalent.

Comment 5

[Stuart Morgan]

Weird; the comment in the patch needs to say as much then, so it's not just changed back later.

Comment 6

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

Richard, can you update this patch?

Comment 7

[R P Mozley]

(In reply to comment #6)
> Richard, can you update this patch?

In what way does it need updating? Has something changed on trunk or some other reason?

Comment 8

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

See comment 5.

Comment 9

[R P Mozley]

I thought Stuart was just agreeing that my comment in the patch was ok. If that's not the case then I'll have to think about how to rephrase as I'm a little muddled about what is needed.

Comment 10

[Stuart Morgan]

The comment needs to make it clear that this form is in fact device dependent despite the claims of the docs.

Comment 11

[R P Mozley]

OK, understood now. I'll have the patch updated sometime, as soon as I can.

Comment 12

[R P Mozley]

Attachment: Updated patch

I've added the extra comments as requested. I hope it meets with your approval.

Comment 13

[Chris Lawson (gone)]

I'm gonna go ahead and confirm this since we have a patch :-p

Comment 14

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

Comment on attachment 414386 [details] [diff] [review]
Updated patch

Even better, the patch needs a reviewer; I'm not sure who's the right person, but Stuart can find another victim if necessary ;-)

Comment 15

[Stuart Morgan]

Comment on attachment 414386 [details] [diff] [review]
Updated patch

r=me.

And come to think of it, I think I remember someone at work running into this "the docs are lying" thing while doing pixel-based unit tests a while back.

Comment 16

[Mike Pinkerton (not reading bugmail)]

Comment on attachment 414386 [details] [diff] [review]
Updated patch

sr=pink

Comment 17

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

Landed on cvs trunk and CAMINO_2_0_BRANCH.  Thanks, Richard!