Incl. support for dppx!
Created attachment 168234 [details] Patch
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 on attachment 168234 [details] Patch Attachment 168234 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/14258621
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 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.
(In reply to comment #5) Sorry, typo. const -> static
Created attachment 168715 [details] Patch
Created attachment 168721 [details] Patch
Comment on attachment 168721 [details] Patch Attachment 168721 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/14300695
Comment on attachment 168721 [details] Patch Attachment 168721 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/14293722
Comment on attachment 168721 [details] Patch Attachment 168721 [details] did not pass cr-android-ews (chromium-android): Output: http://queues.webkit.org/results/14293726
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 on attachment 168721 [details] Patch Attachment 168721 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/14295694
Comment on attachment 168721 [details] Patch Attachment 168721 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/14297741
Created attachment 168914 [details] Patch
Comment on attachment 168914 [details] Patch Attachment 168914 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/14391122
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 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).
(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!
Created attachment 168925 [details] Patch
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 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 on attachment 168925 [details] Patch Attachment 168925 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/14389165
Created attachment 168958 [details] Patch
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 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 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 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
Created attachment 169190 [details] Patch
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 on attachment 169190 [details] Patch Attachment 169190 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/14392626
Created attachment 169200 [details] Patch
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 on attachment 169200 [details] Patch Attachment 169200 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/14384606
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
Created attachment 169228 [details] Patch
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.
Created attachment 169229 [details] Patch
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 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 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 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
Created attachment 169265 [details] Patch
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.
Created attachment 169928 [details] Patch
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.
Created attachment 169969 [details] Patch
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 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?
(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.
Created attachment 169980 [details] Patch
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.
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)
(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 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.
Created attachment 170107 [details] Patch
Created attachment 170139 [details] Patch
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 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
Landed in http://trac.webkit.org/changeset/132227
(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.
Yes, there are other follow up patches landed already which deals with printing. Currently it assumes a printer dpi of minimum 300.