WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
Bug 99077
Add support for resolution media query
https://bugs.webkit.org/show_bug.cgi?id=99077
Summary
Add support for resolution media query
Kenneth Rohde Christiansen
Reported
2012-10-11 08:38:37 PDT
Incl. support for dppx!
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
Show Obsolete
(15)
View All
Add attachment
proposed patch, testcase, etc.
Kenneth Rohde Christiansen
Comment 1
2012-10-11 08:43:08 PDT
Created
attachment 168234
[details]
Patch
WebKit Review Bot
Comment 2
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.
Build Bot
Comment 3
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
WebKit Review Bot
Comment 4
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
Alexander Shalamov
Comment 5
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.
Alexander Shalamov
Comment 6
2012-10-11 23:46:12 PDT
(In reply to
comment #5
) Sorry, typo. const -> static
Kenneth Rohde Christiansen
Comment 7
2012-10-15 08:30:42 PDT
Created
attachment 168715
[details]
Patch
Kenneth Rohde Christiansen
Comment 8
2012-10-15 08:43:52 PDT
Created
attachment 168721
[details]
Patch
Build Bot
Comment 9
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
Build Bot
Comment 10
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
Peter Beverloo (cr-android ews)
Comment 11
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
Eric Seidel (no email)
Comment 12
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.
kov's GTK+ EWS bot
Comment 13
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
WebKit Review Bot
Comment 14
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
Kenneth Rohde Christiansen
Comment 15
2012-10-16 04:00:08 PDT
Created
attachment 168914
[details]
Patch
Build Bot
Comment 16
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
Kenneth Rohde Christiansen
Comment 17
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
Alexis Menard (darktears)
Comment 18
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).
Kenneth Rohde Christiansen
Comment 19
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!
Kenneth Rohde Christiansen
Comment 20
2012-10-16 05:12:42 PDT
Created
attachment 168925
[details]
Patch
WebKit Review Bot
Comment 21
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
Build Bot
Comment 22
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
kov's GTK+ EWS bot
Comment 23
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
Kenneth Rohde Christiansen
Comment 24
2012-10-16 09:09:26 PDT
Created
attachment 168958
[details]
Patch
WebKit Review Bot
Comment 25
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.
Build Bot
Comment 26
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
WebKit Review Bot
Comment 27
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
WebKit Review Bot
Comment 28
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
Kenneth Rohde Christiansen
Comment 29
2012-10-17 08:30:48 PDT
Created
attachment 169190
[details]
Patch
WebKit Review Bot
Comment 30
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.
Build Bot
Comment 31
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
Kenneth Rohde Christiansen
Comment 32
2012-10-17 09:45:52 PDT
Created
attachment 169200
[details]
Patch
WebKit Review Bot
Comment 33
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.
Build Bot
Comment 34
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
WebKit Review Bot
Comment 35
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
Kenneth Rohde Christiansen
Comment 36
2012-10-17 11:49:51 PDT
Created
attachment 169228
[details]
Patch
WebKit Review Bot
Comment 37
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.
Kenneth Rohde Christiansen
Comment 38
2012-10-17 11:55:00 PDT
Created
attachment 169229
[details]
Patch
WebKit Review Bot
Comment 39
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.
Build Bot
Comment 40
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
WebKit Review Bot
Comment 41
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
Build Bot
Comment 42
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
Kenneth Rohde Christiansen
Comment 43
2012-10-17 14:47:35 PDT
Created
attachment 169265
[details]
Patch
WebKit Review Bot
Comment 44
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.
Kenneth Rohde Christiansen
Comment 45
2012-10-22 09:35:38 PDT
Created
attachment 169928
[details]
Patch
WebKit Review Bot
Comment 46
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.
Kenneth Rohde Christiansen
Comment 47
2012-10-22 13:32:10 PDT
Created
attachment 169969
[details]
Patch
WebKit Review Bot
Comment 48
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.
Adam Barth
Comment 49
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?
Kenneth Rohde Christiansen
Comment 50
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.
Kenneth Rohde Christiansen
Comment 51
2012-10-22 14:03:54 PDT
Created
attachment 169980
[details]
Patch
WebKit Review Bot
Comment 52
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.
Kenneth Rohde Christiansen
Comment 53
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)
Kenneth Rohde Christiansen
Comment 54
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
Antti Koivisto
Comment 55
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.
Kenneth Rohde Christiansen
Comment 56
2012-10-23 03:36:59 PDT
Created
attachment 170107
[details]
Patch
Kenneth Rohde Christiansen
Comment 57
2012-10-23 06:00:08 PDT
Created
attachment 170139
[details]
Patch
WebKit Review Bot
Comment 58
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.
WebKit Review Bot
Comment 59
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
Kenneth Rohde Christiansen
Comment 60
2012-10-23 08:16:26 PDT
Landed in
http://trac.webkit.org/changeset/132227
Rick Byers
Comment 61
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.
Kenneth Rohde Christiansen
Comment 62
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.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug