Bug 31373 - rewrite fast/dom/Window/window-screen-properties to be slightly more portable
Summary: rewrite fast/dom/Window/window-screen-properties to be slightly more portable
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Dirk Pranke
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-11-11 14:07 PST by Dirk Pranke
Modified: 2009-11-11 19:02 PST (History)
2 users (show)

See Also:


Attachments
proposed rewrite of patch (167.77 KB, patch)
2009-11-11 14:09 PST, Dirk Pranke
dimich: review-
Details | Formatted Diff | Diff
revised patch - corrects typo in test case, drops mistakenly included pngs (2.47 KB, patch)
2009-11-11 15:40 PST, Dirk Pranke
dimich: review-
Details | Formatted Diff | Diff
drop unnecessary win expectation (3.05 KB, patch)
2009-11-11 17:22 PST, Dirk Pranke
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dirk Pranke 2009-11-11 14:07:07 PST
Right now this test will fail unless your screen is set to a single fixed colordepth (24 on the Mac). Even if we have platform-specific overrides, it means everyone on that platform has to have the same colordepth, which at least for Chromium's infrastructure isn't practical.

I'd like to rewrite this to accept either 16, 24, or 32 as values. That seems like it's enough to ensure the property isn't returning garbage, but is still pretty flexible.
Comment 1 Dirk Pranke 2009-11-11 14:09:15 PST
Created attachment 43000 [details]
proposed rewrite of patch
Comment 2 Dmitry Titov 2009-11-11 14:57:02 PST
Comment on attachment 43000 [details]
proposed rewrite of patch

The patch seems to contain unrelated png files.

Also, while you are at it, does it make sense to verify that pixelDepth==colorDepth or is it not actually always the case?

r- because of extra png files
Comment 3 Dirk Pranke 2009-11-11 15:38:46 PST
grr. Not only were there pngs that snuck in (I'm not sure how), there was a typo in the test file.

per http://www.quirksmode.org/dom/w3c_cssom.html , there may be differences between pixeldepth and colordepth on older unix machines (where bpp <= 8). I don't know if we even support bpp <= 8, so I could certainly add that check if we wanted.
Comment 4 Dirk Pranke 2009-11-11 15:40:48 PST
Created attachment 43013 [details]
revised patch - corrects typo in test case, drops mistakenly included pngs
Comment 5 Dmitry Titov 2009-11-11 16:35:47 PST
Comment on attachment 43013 [details]
revised patch - corrects typo in test case, drops mistakenly included pngs

Almost there. I think you also need to update platform/win/fast/dom/Window/window-screen-properties-expected.txt
Comment 6 Dmitry Titov 2009-11-11 16:39:43 PST
Err, not to update but rather delete since your updated test removes a need for a platform-specific snapshot.
Comment 7 Dirk Pranke 2009-11-11 17:22:22 PST
Created attachment 43021 [details]
drop unnecessary win expectation

Good catch, actually we should be able to delete it.
Comment 8 WebKit Commit Bot 2009-11-11 19:02:23 PST
Comment on attachment 43021 [details]
drop unnecessary win expectation

Clearing flags on attachment: 43021

Committed r50857: <http://trac.webkit.org/changeset/50857>
Comment 9 WebKit Commit Bot 2009-11-11 19:02:31 PST
All reviewed patches have been landed.  Closing bug.