Bug 38116 - Media queries empty values
: Media queries empty values
Status: CLOSED FIXED
: WebKit
WebCore Misc.
: 528+ (Nightly build)
: All All
: P2 Normal
Assigned To:
:
:
:
: 35784
  Show dependency treegraph
 
Reported: 2010-04-26 05:12 PST by
Modified: 2010-05-05 08:11 PST (History)


Attachments
patch 1 (4.56 KB, patch)
2010-04-26 05:28 PST, Luiz Agostini
kenneth: review-
kenneth: commit‑queue-
Review Patch | Details | Formatted Diff | Diff
patch 2 (6.25 KB, application/octet-stream)
2010-04-26 06:59 PST, Luiz Agostini
no flags Details
patch 2 (6.25 KB, patch)
2010-04-26 07:05 PST, Luiz Agostini
no flags Review Patch | Details | Formatted Diff | Diff
patch 3 (6.24 KB, patch)
2010-04-26 07:30 PST, Luiz Agostini
kenneth: review+
commit-queue: commit‑queue-
Review Patch | Details | Formatted Diff | Diff
patch 4 (6.04 KB, patch)
2010-04-27 11:49 PST, Luiz Agostini
simon.fraser: review+
simon.fraser: commit‑queue-
Review Patch | Details | Formatted Diff | Diff
patch 5 (6.04 KB, patch)
2010-04-28 11:34 PST, Luiz Agostini
no flags Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2010-04-26 05:12:29 PST
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.
------- Comment #1 From 2010-04-26 05:16:43 PST -------
(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.
------- Comment #2 From 2010-04-26 05:28:56 PST -------
Created an attachment (id=54281) [details]
patch 1
------- Comment #3 From 2010-04-26 05:33:10 PST -------
(From update of attachment 54281 [details])

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?
------- Comment #4 From 2010-04-26 06:59:41 PST -------
Created an attachment (id=54291) [details]
patch 2
------- Comment #5 From 2010-04-26 07:01:23 PST -------
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.
------- Comment #6 From 2010-04-26 07:05:07 PST -------
Created an attachment (id=54293) [details]
patch 2
------- Comment #7 From 2010-04-26 07:08:40 PST -------
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.
------- Comment #8 From 2010-04-26 07:30:00 PST -------
Created an attachment (id=54297) [details]
patch 3

correcting style error.
------- Comment #9 From 2010-04-26 08:17:38 PST -------
Please provide a link to the spec that describes how such media queries should be processed.
------- Comment #10 From 2010-04-26 09:43:51 PST -------
(From update of attachment 54297 [details])
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
------- Comment #11 From 2010-04-27 11:49:08 PST -------
Created an attachment (id=54436) [details]
patch 4
------- Comment #12 From 2010-04-27 15:45:41 PST -------
(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
------- Comment #13 From 2010-04-27 16:14:01 PST -------
(From update of attachment 54436 [details])
> 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
------- Comment #14 From 2010-04-28 11:34:36 PST -------
Created an attachment (id=54596) [details]
patch 5
------- Comment #15 From 2010-04-29 00:05:41 PST -------
(From update of attachment 54596 [details])
Clearing flags on attachment: 54596

Committed r58479: <http://trac.webkit.org/changeset/58479>
------- Comment #16 From 2010-04-29 00:05:49 PST -------
All reviewed patches have been landed.  Closing bug.
------- Comment #17 From 2010-05-05 08:11:07 PST -------
Revision r58479 cherry-picked into qtwebkit-2.0 with commit 9ee650877afa91d7941d98803d1fb23840201c9b