Bug 99077 - Add support for resolution media query
Summary: Add support for resolution media query
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Kenneth Rohde Christiansen
URL: http://www.w3.org/TR/css3-mediaquerie...
Keywords: WebExposed
Depends on: 100865
Blocks: 78087 100861
  Show dependency treegraph
 
Reported: 2012-10-11 08:38 PDT by Kenneth Rohde Christiansen
Modified: 2013-09-09 14:04 PDT (History)
28 users (show)

See Also:


Attachments
Patch (15.10 KB, patch)
2012-10-11 08:43 PDT, Kenneth Rohde Christiansen
no flags Details | Formatted Diff | Diff
Patch (25.94 KB, patch)
2012-10-15 08:30 PDT, Kenneth Rohde Christiansen
no flags Details | Formatted Diff | Diff
Patch (26.45 KB, patch)
2012-10-15 08:43 PDT, Kenneth Rohde Christiansen
no flags Details | Formatted Diff | Diff
Patch (28.16 KB, patch)
2012-10-16 04:00 PDT, Kenneth Rohde Christiansen
no flags Details | Formatted Diff | Diff
Patch (30.23 KB, patch)
2012-10-16 05:12 PDT, Kenneth Rohde Christiansen
no flags Details | Formatted Diff | Diff
Patch (30.23 KB, patch)
2012-10-16 09:09 PDT, Kenneth Rohde Christiansen
no flags Details | Formatted Diff | Diff
Patch (32.46 KB, patch)
2012-10-17 08:30 PDT, Kenneth Rohde Christiansen
no flags Details | Formatted Diff | Diff
Patch (32.49 KB, patch)
2012-10-17 09:45 PDT, Kenneth Rohde Christiansen
no flags Details | Formatted Diff | Diff
Patch (32.54 KB, patch)
2012-10-17 11:49 PDT, Kenneth Rohde Christiansen
no flags Details | Formatted Diff | Diff
Patch (32.56 KB, patch)
2012-10-17 11:55 PDT, Kenneth Rohde Christiansen
no flags Details | Formatted Diff | Diff
Patch (32.57 KB, patch)
2012-10-17 14:47 PDT, Kenneth Rohde Christiansen
no flags Details | Formatted Diff | Diff
Patch (34.23 KB, patch)
2012-10-22 09:35 PDT, Kenneth Rohde Christiansen
no flags Details | Formatted Diff | Diff
Patch (35.65 KB, patch)
2012-10-22 13:32 PDT, Kenneth Rohde Christiansen
no flags Details | Formatted Diff | Diff
Patch (35.36 KB, patch)
2012-10-22 14:03 PDT, Kenneth Rohde Christiansen
no flags Details | Formatted Diff | Diff
Patch (8.56 KB, patch)
2012-10-23 03:36 PDT, Kenneth Rohde Christiansen
no flags Details | Formatted Diff | Diff
Patch (59.13 KB, patch)
2012-10-23 06:00 PDT, Kenneth Rohde Christiansen
koivisto: review+
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kenneth Rohde Christiansen 2012-10-11 08:38:37 PDT
Incl. support for dppx!
Comment 1 Kenneth Rohde Christiansen 2012-10-11 08:43:08 PDT
Created attachment 168234 [details]
Patch
Comment 2 WebKit Review Bot 2012-10-11 09:36:05 PDT
Attachment 168234 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast..." exit_code: 1
Source/WebCore/css/MediaQueryExp.cpp:163:  Extra space before ( in function call  [whitespace/parens] [4]
Source/WebCore/css/MediaQueryEvaluator.cpp:507:  min_resolutionMediaFeatureEval is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Source/WebCore/css/MediaQueryEvaluator.cpp:512:  max_resolutionMediaFeatureEval is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 3 in 11 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Build Bot 2012-10-11 10:08:08 PDT
Comment on attachment 168234 [details]
Patch

Attachment 168234 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/14258621
Comment 4 WebKit Review Bot 2012-10-11 10:51:13 PDT
Comment on attachment 168234 [details]
Patch

Attachment 168234 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/14248984

New failing tests:
fast/media/mq-resolution.html
Comment 5 Alexander Shalamov 2012-10-11 23:45:21 PDT
Comment on attachment 168234 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=168234&action=review

> Source/WebCore/css/MediaQueryEvaluator.cpp:200
> +bool compareResolution(unsigned min, unsigned max, unsigned value, MediaFeaturePrefix op)

Should it be const, so that it will stay in this compilation unit. Maybe that's why mac build fails.

/Volumes/Data/EWS/WebKit/Source/WebCore/css/MediaQueryEvaluator.cpp:200:6: error: no previous prototype for function 'compareResolution' [-Werror,-Wmissing-prototypes]
bool compareResolution(unsigned min, unsigned max, unsigned value, MediaFeaturePrefix op)
     ^
1 error generated.
Comment 6 Alexander Shalamov 2012-10-11 23:46:12 PDT
(In reply to comment #5)
Sorry, typo. const -> static
Comment 7 Kenneth Rohde Christiansen 2012-10-15 08:30:42 PDT
Created attachment 168715 [details]
Patch
Comment 8 Kenneth Rohde Christiansen 2012-10-15 08:43:52 PDT
Created attachment 168721 [details]
Patch
Comment 9 Build Bot 2012-10-15 09:40:02 PDT
Comment on attachment 168721 [details]
Patch

Attachment 168721 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/14300695
Comment 10 Build Bot 2012-10-15 09:42:00 PDT
Comment on attachment 168721 [details]
Patch

Attachment 168721 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/14293722
Comment 11 Peter Beverloo (cr-android ews) 2012-10-15 09:55:04 PDT
Comment on attachment 168721 [details]
Patch

Attachment 168721 [details] did not pass cr-android-ews (chromium-android):
Output: http://queues.webkit.org/results/14293726
Comment 12 Eric Seidel (no email) 2012-10-15 14:01:47 PDT
Attachment 168721 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast..." exit_code: 1
Source/WebCore/css/MediaQueryEvaluator.cpp:51:  Alphabetical sorting problem.  [build/include_order] [4]
Source/WebCore/css/MediaQueryEvaluator.cpp:523:  min_resolutionMediaFeatureEval is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Source/WebCore/css/MediaQueryEvaluator.cpp:528:  max_resolutionMediaFeatureEval is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 3 in 14 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 13 kov's GTK+ EWS bot 2012-10-15 14:02:55 PDT
Comment on attachment 168721 [details]
Patch

Attachment 168721 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/14295694
Comment 14 WebKit Review Bot 2012-10-15 15:04:40 PDT
Comment on attachment 168721 [details]
Patch

Attachment 168721 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/14297741
Comment 15 Kenneth Rohde Christiansen 2012-10-16 04:00:08 PDT
Created attachment 168914 [details]
Patch
Comment 16 Build Bot 2012-10-16 04:41:03 PDT
Comment on attachment 168914 [details]
Patch

Attachment 168914 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/14391122
Comment 17 Kenneth Rohde Christiansen 2012-10-16 04:49:48 PDT
Any clue anyone? 

Undefined symbols for architecture x86_64:
  "__ZN7WebCore8Settings21setResolutionOverrideERKNS_7IntRectE", referenced from:
     -exported_symbol[s_list] command line option
ld: symbol(s) not found for architecture x86_64
Comment 18 Alexis Menard (darktears) 2012-10-16 04:55:24 PDT
Comment on attachment 168914 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=168914&action=review

> Source/WebCore/page/Settings.cpp:462
> +void Settings::setResolutionOverride(const IntSize& densityPerInchOverride)

I think the declaration doesn't match with your Source/WebCore/WebCore.exp.in declaration (IntSize -> IntRect).
Comment 19 Kenneth Rohde Christiansen 2012-10-16 04:55:49 PDT
(In reply to comment #17)
> Any clue anyone? 
> 
> Undefined symbols for architecture x86_64:
>   "__ZN7WebCore8Settings21setResolutionOverrideERKNS_7IntRectE", referenced from:
>      -exported_symbol[s_list] command line option
> ld: symbol(s) not found for architecture x86_64

s/IntRect/IntSize :-) need more coffee!
Comment 20 Kenneth Rohde Christiansen 2012-10-16 05:12:42 PDT
Created attachment 168925 [details]
Patch
Comment 21 WebKit Review Bot 2012-10-16 06:31:05 PDT
Comment on attachment 168925 [details]
Patch

Attachment 168925 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/14396115

New failing tests:
fast/media/mq-resolution.html
Comment 22 Build Bot 2012-10-16 07:16:59 PDT
Comment on attachment 168925 [details]
Patch

Attachment 168925 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/14391165

New failing tests:
fast/media/mq-resolution.html
Comment 23 kov's GTK+ EWS bot 2012-10-16 07:22:28 PDT
Comment on attachment 168925 [details]
Patch

Attachment 168925 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/14389165
Comment 24 Kenneth Rohde Christiansen 2012-10-16 09:09:26 PDT
Created attachment 168958 [details]
Patch
Comment 25 WebKit Review Bot 2012-10-16 10:09:37 PDT
Attachment 168958 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'ChangeLog', u'LayoutTests/ChangeLog', u'La..." exit_code: 1
Source/WebCore/css/MediaQueryEvaluator.cpp:52:  Alphabetical sorting problem.  [build/include_order] [4]
Source/WebCore/css/MediaQueryEvaluator.cpp:524:  min_resolutionMediaFeatureEval is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Source/WebCore/css/MediaQueryEvaluator.cpp:529:  max_resolutionMediaFeatureEval is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 3 in 19 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 26 Build Bot 2012-10-16 10:22:07 PDT
Comment on attachment 168958 [details]
Patch

Attachment 168958 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/14386210

New failing tests:
fast/media/mq-resolution.html
Comment 27 WebKit Review Bot 2012-10-16 12:40:41 PDT
Comment on attachment 168958 [details]
Patch

Attachment 168958 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/14396235

New failing tests:
fast/media/mq-resolution.html
Comment 28 WebKit Review Bot 2012-10-16 13:41:53 PDT
Comment on attachment 168958 [details]
Patch

Attachment 168958 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/14384297

New failing tests:
fast/media/mq-resolution.html
Comment 29 Kenneth Rohde Christiansen 2012-10-17 08:30:48 PDT
Created attachment 169190 [details]
Patch
Comment 30 WebKit Review Bot 2012-10-17 08:33:01 PDT
Attachment 169190 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'ChangeLog', u'LayoutTests/ChangeLog', u'La..." exit_code: 1
LayoutTests/platform/chromium/TestExpectations:174:  Path does not exist.  [test/expectations] [5]
LayoutTests/platform/mac/TestExpectations:968:  Path does not exist.  [test/expectations] [5]
Source/WebCore/css/MediaQueryEvaluator.cpp:52:  Alphabetical sorting problem.  [build/include_order] [4]
Source/WebCore/css/MediaQueryEvaluator.cpp:526:  min_resolutionMediaFeatureEval is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Source/WebCore/css/MediaQueryEvaluator.cpp:531:  max_resolutionMediaFeatureEval is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 5 in 21 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 31 Build Bot 2012-10-17 09:06:16 PDT
Comment on attachment 169190 [details]
Patch

Attachment 169190 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/14392626
Comment 32 Kenneth Rohde Christiansen 2012-10-17 09:45:52 PDT
Created attachment 169200 [details]
Patch
Comment 33 WebKit Review Bot 2012-10-17 09:50:12 PDT
Attachment 169200 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'ChangeLog', u'LayoutTests/ChangeLog', u'La..." exit_code: 1
LayoutTests/platform/chromium/TestExpectations:174:  Path does not exist.  [test/expectations] [5]
LayoutTests/platform/mac/TestExpectations:969:  Path does not exist.  [test/expectations] [5]
Source/WebCore/css/MediaQueryEvaluator.cpp:52:  Alphabetical sorting problem.  [build/include_order] [4]
Source/WebCore/css/MediaQueryEvaluator.cpp:528:  min_resolutionMediaFeatureEval is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Source/WebCore/css/MediaQueryEvaluator.cpp:533:  max_resolutionMediaFeatureEval is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 5 in 21 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 34 Build Bot 2012-10-17 10:23:24 PDT
Comment on attachment 169200 [details]
Patch

Attachment 169200 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/14384606
Comment 35 WebKit Review Bot 2012-10-17 11:07:18 PDT
Comment on attachment 169200 [details]
Patch

Attachment 169200 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/14392656

New failing tests:
fast/media/mq-resolution.html
Comment 36 Kenneth Rohde Christiansen 2012-10-17 11:49:51 PDT
Created attachment 169228 [details]
Patch
Comment 37 WebKit Review Bot 2012-10-17 11:52:23 PDT
Attachment 169228 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'ChangeLog', u'LayoutTests/ChangeLog', u'La..." exit_code: 1
LayoutTests/platform/chromium/TestExpectations:174:  Path does not exist.  [test/expectations] [5]
LayoutTests/platform/mac/TestExpectations:969:  Path does not exist.  [test/expectations] [5]
Source/WebCore/css/MediaQueryEvaluator.cpp:52:  Alphabetical sorting problem.  [build/include_order] [4]
Source/WebCore/css/MediaQueryEvaluator.cpp:530:  min_resolutionMediaFeatureEval is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Source/WebCore/css/MediaQueryEvaluator.cpp:535:  max_resolutionMediaFeatureEval is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 5 in 21 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 38 Kenneth Rohde Christiansen 2012-10-17 11:55:00 PDT
Created attachment 169229 [details]
Patch
Comment 39 WebKit Review Bot 2012-10-17 11:57:51 PDT
Attachment 169229 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'ChangeLog', u'LayoutTests/ChangeLog', u'La..." exit_code: 1
LayoutTests/platform/chromium/TestExpectations:174:  Path does not exist.  [test/expectations] [5]
LayoutTests/platform/mac/TestExpectations:969:  Path does not exist.  [test/expectations] [5]
Source/WebCore/css/MediaQueryEvaluator.cpp:52:  Alphabetical sorting problem.  [build/include_order] [4]
Source/WebCore/css/MediaQueryEvaluator.cpp:530:  min_resolutionMediaFeatureEval is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Source/WebCore/css/MediaQueryEvaluator.cpp:535:  max_resolutionMediaFeatureEval is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 5 in 21 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 40 Build Bot 2012-10-17 13:20:23 PDT
Comment on attachment 169229 [details]
Patch

Attachment 169229 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/14387689

New failing tests:
fast/media/mq-resolution.html
Comment 41 WebKit Review Bot 2012-10-17 13:24:38 PDT
Comment on attachment 169229 [details]
Patch

Attachment 169229 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/14388708

New failing tests:
fast/media/mq-resolution.html
Comment 42 Build Bot 2012-10-17 14:20:18 PDT
Comment on attachment 169229 [details]
Patch

Attachment 169229 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/14390683

New failing tests:
fast/media/mq-resolution.html
Comment 43 Kenneth Rohde Christiansen 2012-10-17 14:47:35 PDT
Created attachment 169265 [details]
Patch
Comment 44 WebKit Review Bot 2012-10-17 14:50:59 PDT
Attachment 169265 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'ChangeLog', u'LayoutTests/ChangeLog', u'La..." exit_code: 1
Source/WebCore/css/MediaQueryEvaluator.cpp:52:  Alphabetical sorting problem.  [build/include_order] [4]
Source/WebCore/css/MediaQueryEvaluator.cpp:530:  min_resolutionMediaFeatureEval is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Source/WebCore/css/MediaQueryEvaluator.cpp:535:  max_resolutionMediaFeatureEval is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 3 in 21 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 45 Kenneth Rohde Christiansen 2012-10-22 09:35:38 PDT
Created attachment 169928 [details]
Patch
Comment 46 WebKit Review Bot 2012-10-22 09:39:38 PDT
Attachment 169928 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'ChangeLog', u'LayoutTests/ChangeLog', u'La..." exit_code: 1
Source/WebCore/css/MediaQueryEvaluator.cpp:52:  Alphabetical sorting problem.  [build/include_order] [4]
Source/WebCore/css/MediaQueryEvaluator.cpp:557:  min_resolutionMediaFeatureEval is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Source/WebCore/css/MediaQueryEvaluator.cpp:562:  max_resolutionMediaFeatureEval is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 3 in 21 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 47 Kenneth Rohde Christiansen 2012-10-22 13:32:10 PDT
Created attachment 169969 [details]
Patch
Comment 48 WebKit Review Bot 2012-10-22 13:35:48 PDT
Attachment 169969 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'ChangeLog', u'LayoutTests/ChangeLog', u'La..." exit_code: 1
Source/WebCore/css/MediaQueryEvaluator.cpp:557:  min_resolutionMediaFeatureEval is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Source/WebCore/css/MediaQueryEvaluator.cpp:562:  max_resolutionMediaFeatureEval is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 2 in 21 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 49 Adam Barth 2012-10-22 13:46:04 PDT
Comment on attachment 169969 [details]
Patch

I don't see a link to a spec in the ChangeLog.  Is this part of a spec anywhere?  Also, you might want to link to the webkit-dev thread described in http://www.webkit.org/coding/adding-features.html.  Finally, isn't there already a media query for device pixel ratio?
Comment 50 Kenneth Rohde Christiansen 2012-10-22 13:51:15 PDT
(In reply to comment #49)
> (From update of attachment 169969 [details])
> I don't see a link to a spec in the ChangeLog.  Is this part of a spec anywhere?  Also, you might want to link to the webkit-dev thread described in http://www.webkit.org/coding/adding-features.html.  Finally, isn't there already a media query for device pixel ratio?

I will add that; they are:

http://www.w3.org/TR/css3-mediaqueries/#resolution
http://dev.w3.org/csswg/css3-values/#resolution

Yes, basically it was discussed to push device pixel ratio to become spec'ed but resolution was already specced and implemented by other, like Mozilla. As (min-resolution: 192) is harder to understand than (device-pixel-ratio: 2) the dppx syntax was added in css3-values, which represents the same.
Comment 51 Kenneth Rohde Christiansen 2012-10-22 14:03:54 PDT
Created attachment 169980 [details]
Patch
Comment 52 WebKit Review Bot 2012-10-22 14:06:50 PDT
Attachment 169980 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'ChangeLog', u'LayoutTests/ChangeLog', u'La..." exit_code: 1
Source/WebCore/css/MediaQueryEvaluator.cpp:557:  min_resolutionMediaFeatureEval is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Source/WebCore/css/MediaQueryEvaluator.cpp:562:  max_resolutionMediaFeatureEval is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 2 in 21 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 53 Kenneth Rohde Christiansen 2012-10-23 02:03:46 PDT
FYI, this features (incl. dppx) is shipped by Mozilla in Firefox 16. Opera are also shipping the 'resolution' media query, but not with dppx support yet (at least to my knowledge)
Comment 54 Kenneth Rohde Christiansen 2012-10-23 02:05:00 PDT
(In reply to comment #53)
> FYI, this features (incl. dppx) is shipped by Mozilla in Firefox 16. Opera are also shipping the 'resolution' media query, but not with dppx support yet (at least to my knowledge)

Actually Opera has it implemented, but are not shipping it yet: https://twitter.com/frivoal/status/231420753130692608
Comment 55 Antti Koivisto 2012-10-23 02:12:54 PDT
Comment on attachment 169980 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=169980&action=review

> Source/WebCore/css/MediaQueryEvaluator.cpp:201
> +#if ENABLE(CSS_IMAGE_RESOLUTION)
> +static bool compareResolution(float min, float max, float value, MediaFeaturePrefix op)

It seems bit surprising to tie this to CSS image resolution feature. Maybe the new units could be enabled globally (they shouldn't hurt even without clients) and this feature could have a feature flag of its own.
Comment 56 Kenneth Rohde Christiansen 2012-10-23 03:36:59 PDT
Created attachment 170107 [details]
Patch
Comment 57 Kenneth Rohde Christiansen 2012-10-23 06:00:08 PDT
Created attachment 170139 [details]
Patch
Comment 58 WebKit Review Bot 2012-10-23 06:03:10 PDT
Attachment 170139 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'ChangeLog', u'LayoutTests/ChangeLog', u'La..." exit_code: 1
Source/WebCore/css/MediaQueryEvaluator.cpp:557:  min_resolutionMediaFeatureEval is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Source/WebCore/css/MediaQueryEvaluator.cpp:562:  max_resolutionMediaFeatureEval is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 2 in 35 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 59 WebKit Review Bot 2012-10-23 07:51:55 PDT
Comment on attachment 170139 [details]
Patch

Rejecting attachment 170139 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2

Last 500 characters of output:
T (content): Merge conflict in Source/WebKit2/ChangeLog
Failed to merge in the changes.
Patch failed at 0002 [EFL][WK2] Convert WebEvent's timestamp from millisecond to second.

When you have resolved this problem run "git rebase --continue".
If you would prefer to skip this patch, instead run "git rebase --skip".
To restore the original branch and stop rebasing run "git rebase --abort".

rebase refs/remotes/origin/master: command returned error: 1

Died at Tools/Scripts/update-webkit line 164.

Full output: http://queues.webkit.org/results/14487910
Comment 60 Kenneth Rohde Christiansen 2012-10-23 08:16:26 PDT
Landed in http://trac.webkit.org/changeset/132227
Comment 61 Rick Byers 2012-10-31 07:52:30 PDT
(In reply to comment #60)
> Landed in http://trac.webkit.org/changeset/132227

Thanks for doing this Kenneth!  I've been thinking we want this in chromium as well, but had hesitated doing it myself because I wasn't sure if it would be difficult to get it to work correctly for print media.  Did you look at the print media cases at all?  I'm not an expert in the relevant code, but the computation of DPI strictly from deviceScaleFactor makes me think it might not do the right thing for print.
Comment 62 Kenneth Rohde Christiansen 2012-10-31 09:06:05 PDT
Yes, there are other follow up patches landed already which deals with printing. Currently it assumes a printer dpi of minimum 300.