Summary: | Make CSSMedia queries pass through Chrome and ChromeClient | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Robert Hogan <robert> | ||||||||
Component: | New Bugs | Assignee: | Robert Hogan <robert> | ||||||||
Status: | RESOLVED WONTFIX | ||||||||||
Severity: | Normal | CC: | abarth, cmarcelo, hyatt, mikeperry-webkit, robert | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Other | ||||||||||
OS: | OS X 10.5 | ||||||||||
Bug Depends on: | |||||||||||
Bug Blocks: | 50895, 56678 | ||||||||||
Attachments: |
|
Description
Robert Hogan
2011-03-16 13:22:44 PDT
Created attachment 85963 [details]
Patch
Comment on attachment 85963 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=85963&action=review Why? Please explain in your ChangeLog. > Source/WebCore/css/MediaQueryEvaluator.cpp:-232 > - if (!screenIsMonochrome(frame->page()->mainFrame()->view())) { what happens to the screenIsMonochrome function? Is it being removed? Created attachment 86050 [details]
Patch
Comment on attachment 86050 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=86050&action=review > Source/WebCore/css/MediaQueryEvaluator.cpp:232 > + if (!frame->page()->chrome()->screenIsMonochrome()) { No, it's not removed. It's called from Chrome instead. Comment on attachment 86050 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=86050&action=review > Source/WebCore/css/MediaQueryEvaluator.cpp:257 > + FloatRect sg = frame->page()->chrome()->pageRect(); Note to self: this should be windowRect() I think. > Source/WebCore/css/MediaQueryEvaluator.cpp:332 > + FloatRect sg = frame->page()->chrome()->pageRect(); Note to self: this should be windowRect() I think. Comment on attachment 86050 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=86050&action=review > Source/WebCore/page/ChromeClient.h:86 > + int screenDepthPerComponent(int bitsPerComponent) { return bitsPerComponent; } > + bool screenIsMonochrome(bool isMonochrome) { return isMonochrome; } > + FloatRect screenRect(FloatRect rect) { return rect; } > + FloatRect screenAvailableRect(FloatRect rect) { return rect; } Oh, these should be virtual. Comment on attachment 86050 [details]
Patch
So is this only for fighting fingerprinting? Shouldn't anti-fingerprinting measures be in WebCore directly?
(In reply to comment #7) > (From update of attachment 86050 [details]) > So is this only for fighting fingerprinting? Shouldn't anti-fingerprinting measures be in WebCore directly? It isn't always the right place for them. See https://trac.webkit.org/wiki/Fingerprinting#CreatingaCommonFingerprint A browser needs the flexibility to create a common fingerprint for it's users - WebCore isn't always best placed to decide what this fingerprint should be. I think the 'bare metal' properties of the screen and viewport inspected by CSS media queries in PlatformScreen are a good example. A browser will often have to make a trade-off between user experience and a relatively common set of values for screen/chrome/window dimensions - it ensures these are respected by the Screen and Window DOM objects, and it needs to do the same with CSS media queries. A good example of this is in the Torbutton design document referenced here: https://trac.webkit.org/wiki/Fingerprinting#WindowObject Created attachment 86217 [details]
Patch
Comment on attachment 86217 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=86217&action=review > Source/WebCore/css/MediaQueryEvaluator.cpp:222 > - int bitsPerComponent = screenDepthPerComponent(frame->page()->mainFrame()->view()); > + int bitsPerComponent = frame->page()->chrome()->screenDepthPerComponent(); What if page is null? Comment on attachment 86217 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=86217&action=review I disagree with this approach. If we are doing this for fingerprinting avoidance, we should write the anti-fingerprinting code in WebCore instead of piping everything through WebKit. >> Source/WebCore/css/MediaQueryEvaluator.cpp:222 >> + int bitsPerComponent = frame->page()->chrome()->screenDepthPerComponent(); > > What if page is null? frame->page() can only be null during teardown, or? |