466669 - Windows nsIScreen::GetColorDepth() should correct 32 to 24

Status: RESOLVED FIXED

(Core :: Widget: Win32, defect)

Description

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

In bug 424386, we discussed making the Windows widget code correct color depth values that are 32 to be 24 instead.  This is required to follow the spec, as cited in 424386 comment 3.

It wasn't done there because of concerns for Web compatibility.  However, I think the Web compatibility concerns are not relevant because it actually seems to vary from machine to machine -- some display drivers return 24, and some return 32.  (Otherwise bug 424386 would have been a problem for a lot more people, including the tinderboxes.)

So I think we should make the Windows widget code do the correction from 32 to 24 and remove the hack in nsMediaFeatures.cpp's GetDepth function.

Comment 1

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Sorry, my second bug reference should have been bug 458847.  (And I meant for bug 424386 comment 3 to be a link as well.)

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch

Not yet tested or compiled.

Comment 3

[Serge Gautherie (:sgautherie)]

I had just found where to fix this;
but I didn't know about the existing override ;-)

Could you include my bug 424386 comment 18 nit too ?

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch

Comment 5

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Comment on attachment 353515 [details] [diff] [review]
patch

Isn't there a reftest that explicitly tests this?

Comment 6

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Yeah, but we changed it because it was returning 32 for some people; see bug 458847.  Do you think i should undo the change we made there along with this?

Comment 7

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Fixed: http://hg.mozilla.org/mozilla-central/rev/29a3a6c1565a

Comment 8

[Serge Gautherie (:sgautherie)]

(In reply to comment #6)
> Do you think i should undo the change we made there along with this?

I would think the new test is fine, per bug 458847 comment 9 rationale.

Comment 9

[Eric Shepherd [:sheppy]]

In the context of nsIScreen, what's the difference between pixel depth and color depth? The code for both Windows and Mac OS X return the same value for both, so I'm unclear how to document these.

Comment 10

[Eric Shepherd [:sheppy]]

I've written a doc for nsIScreen; I still would like to know what to say about the difference between colorDepth and pixelDepth, but otherwise this is written up, including a note about the change in this bug.

https://developer.mozilla.org/en/XPCOM_Interface_Reference/nsIScreen