CLOSED FIXED 38116
Media queries empty values
https://bugs.webkit.org/show_bug.cgi?id=38116
Summary Media queries empty values
Luiz Agostini
Reported 2010-04-26 05:12:29 PDT
In webkit, invalid media query values are ignored and treated as empty values. For example (-webkit-view-mode) and (-webkit-view-mode:blah) will both be evaluated the same although blah is not a valid view mode value.
Attachments
patch 1 (4.56 KB, patch)
2010-04-26 05:28 PDT, Luiz Agostini
kenneth: review-
kenneth: commit-queue-
patch 2 (6.25 KB, application/octet-stream)
2010-04-26 06:59 PDT, Luiz Agostini
no flags
patch 2 (6.25 KB, patch)
2010-04-26 07:05 PDT, Luiz Agostini
no flags
patch 3 (6.24 KB, patch)
2010-04-26 07:30 PDT, Luiz Agostini
kenneth: review+
commit-queue: commit-queue-
patch 4 (6.04 KB, patch)
2010-04-27 11:49 PDT, Luiz Agostini
simon.fraser: review+
simon.fraser: commit-queue-
patch 5 (6.04 KB, patch)
2010-04-28 11:34 PDT, Luiz Agostini
no flags
Kenneth Rohde Christiansen
Comment 1 2010-04-26 05:16:43 PDT
(In reply to comment #0) > In webkit, invalid media query values are ignored and treated as empty values. > For example (-webkit-view-mode) and (-webkit-view-mode:blah) will both be > evaluated the same although blah is not a valid view mode value. Just to make it clear, the blah is a no-accepted keyword.
Luiz Agostini
Comment 2 2010-04-26 05:28:56 PDT
Created attachment 54281 [details] patch 1
Kenneth Rohde Christiansen
Comment 3 2010-04-26 05:33:10 PDT
Comment on attachment 54281 [details] patch 1 WebCore/ChangeLog:19 + corrected here. After landing this patch it will. The following corrects the view I dont think you should add things like "after landing this patch" in the ChangeLog WebCore/css/CSSValueKeywords.in:718 + windowed This belongs to another patch. It is not related. WebCore/css/MediaQueryExp.h:55 + bool valueParseError() const { return m_valueParseError; } I do not like the name of the method. What about valueIsValid() or so?
Luiz Agostini
Comment 4 2010-04-26 06:59:41 PDT
Created attachment 54291 [details] patch 2
WebKit Review Bot
Comment 5 2010-04-26 07:01:23 PDT
Attachment 54291 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1 WebCore/css/MediaQueryExp.cpp:77: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] Total errors found: 1 in 7 files If any of these errors are false positives, please file a bug against check-webkit-style.
Luiz Agostini
Comment 6 2010-04-26 07:05:07 PDT
Created attachment 54293 [details] patch 2
WebKit Review Bot
Comment 7 2010-04-26 07:08:40 PDT
Attachment 54293 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1 WebCore/css/MediaQueryExp.cpp:77: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] Total errors found: 1 in 7 files If any of these errors are false positives, please file a bug against check-webkit-style.
Luiz Agostini
Comment 8 2010-04-26 07:30:00 PDT
Created attachment 54297 [details] patch 3 correcting style error.
Simon Fraser (smfr)
Comment 9 2010-04-26 08:17:38 PDT
Please provide a link to the spec that describes how such media queries should be processed.
WebKit Commit Bot
Comment 10 2010-04-26 09:43:51 PDT
Comment on attachment 54297 [details] patch 3 Rejecting patch 54297 from commit-queue. Failed to run "['WebKitTools/Scripts/run-webkit-tests', '--no-launch-safari', '--exit-after-n-failures=1', '--quiet']" exit_code: 1 Running build-dumprendertree Compiling Java tests make: Nothing to be done for `default'. Running tests from /Users/eseidel/Projects/CommitQueue/LayoutTests Testing 12730 test cases. fast/media/media-query-invalid-value.html -> failed Exiting early after 1 failures. 7902 tests run. 142.89s total testing time 7901 test cases (99%) succeeded 1 test case (<1%) had incorrect layout 4 test cases (<1%) had stderr output Full output: http://webkit-commit-queue.appspot.com/results/1816138
Luiz Agostini
Comment 11 2010-04-27 11:49:08 PDT
Created attachment 54436 [details] patch 4
Luiz Agostini
Comment 12 2010-04-27 15:45:41 PDT
(In reply to comment #9) > Please provide a link to the spec that describes how such media queries should > be processed. The spec is at http://www.w3.org/TR/css3-mediaqueries/#error-handling
Simon Fraser (smfr)
Comment 13 2010-04-27 16:14:01 PDT
Comment on attachment 54436 [details] patch 4 > diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog > index d2e0b8c..22cf7d4 100644 > --- a/LayoutTests/ChangeLog > +++ b/LayoutTests/ChangeLog > @@ -1,3 +1,16 @@ > +2010-04-27 Luiz Agostini <luiz.agostini@openbossa.org> > + > + Reviewed by NOBODY (OOPS!). > + > + Media queries empty values > + https://bugs.webkit.org/show_bug.cgi?id=38116 > + > + Adding isValid() method to MediaQueryExp to be possible to differentiate > + between queries with empty values and queries with invalid values. .. to make it possible to differentiate... r=me
Luiz Agostini
Comment 14 2010-04-28 11:34:36 PDT
Created attachment 54596 [details] patch 5
WebKit Commit Bot
Comment 15 2010-04-29 00:05:41 PDT
Comment on attachment 54596 [details] patch 5 Clearing flags on attachment: 54596 Committed r58479: <http://trac.webkit.org/changeset/58479>
WebKit Commit Bot
Comment 16 2010-04-29 00:05:49 PDT
All reviewed patches have been landed. Closing bug.
Simon Hausmann
Comment 17 2010-05-05 08:11:07 PDT
Revision r58479 cherry-picked into qtwebkit-2.0 with commit 9ee650877afa91d7941d98803d1fb23840201c9b
Note You need to log in before you can comment on or make changes to this bug.