143262 – Invalid media feature evaluates to true

RESOLVED FIXED Bug 143262

Invalid media feature evaluates to true

https://bugs.webkit.org/show_bug.cgi?id=143262

Summary Invalid media feature evaluates to true

Simon Pieters (:zcorpan)

Reported 2015-03-31 06:21:16 PDT

See http://software.hixie.ch/utilities/js/live-dom-viewer/saved/3473 <!DOCTYPE html> <link rel=stylesheet href="data:text/css,p { color:red }" media="(width invalid)"> <p>There should be no red The stylesheet is incorrectly applied. Blink/Gecko/IE11 do not have this issue. This is an issue for introducing comparison operators in media features, since WebKit will just evaluate them to true. e.g. (width < 500px) https://drafts.csswg.org/mediaqueries/#mq-range-context https://drafts.csswg.org/mediaqueries/#error-handling

Attachments
Patch (3.84 KB, patch) 2015-03-31 09:54 PDT, Yoav Weiss	no flags	Details Formatted Diff Diff
Archive of layout-test-results from ews105 for mac-mavericks-wk2 (254.59 KB, application/zip) 2015-03-31 10:23 PDT, Build Bot	no flags	Details
Archive of layout-test-results from ews103 for mac-mavericks (233.66 KB, application/zip) 2015-03-31 10:42 PDT, Build Bot	no flags	Details
Patch (5.08 KB, patch) 2015-03-31 14:19 PDT, Yoav Weiss	no flags	Details Formatted Diff Diff
Archive of layout-test-results from ews106 for mac-mavericks-wk2 (648.94 KB, application/zip) 2015-03-31 14:44 PDT, Build Bot	no flags	Details
Archive of layout-test-results from ews101 for mac-mavericks (537.73 KB, application/zip) 2015-03-31 15:07 PDT, Build Bot	no flags	Details
Patch (6.77 KB, patch) 2015-04-01 13:36 PDT, Yoav Weiss	no flags	Details Formatted Diff Diff
Patch (6.80 KB, patch) 2016-03-11 00:23 PST, Yoav Weiss	no flags	Details Formatted Diff Diff
Show Obsolete (7) View All Add attachment proposed patch, testcase, etc.

Simon Pieters (:zcorpan)

Comment 1 2015-03-31 07:26:43 PDT

This does not happen for @media or matchMedia. http://software.hixie.ch/utilities/js/live-dom-viewer/saved/3474 passes http://software.hixie.ch/utilities/js/live-dom-viewer/saved/3475 passes

Yoav Weiss

Comment 2 2015-03-31 09:51:31 PDT

*** Bug 143266 has been marked as a duplicate of this bug. ***

Yoav Weiss

Comment 3 2015-03-31 09:54:54 PDT

Created attachment 249827 [details] Patch

Yoav Weiss

Comment 4 2015-03-31 09:56:03 PDT

When <link rel=stylesheet> with an invalid media attribute was part of the HTML, I saw that it resulted in a non-empty MediaQuerySet, containing a single query with no expressions. (which didn't have its m_ignored flag on) Therefore I added an extra check that makes sure that the queries handled by MediaQueryEvaluator::eval have expressions, and if not, they are treated as ignored.

Build Bot

Comment 5 2015-03-31 10:23:10 PDT

Comment on attachment 249827 [details] Patch Attachment 249827 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/6568812564447232 Number of test failures exceeded the failure limit.

Build Bot

Comment 6 2015-03-31 10:23:14 PDT

Created attachment 249829 [details] Archive of layout-test-results from ews105 for mac-mavericks-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews105 Port: mac-mavericks-wk2 Platform: Mac OS X 10.9.5

Build Bot

Comment 7 2015-03-31 10:42:42 PDT

Comment on attachment 249827 [details] Patch Attachment 249827 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/6645182988550144 Number of test failures exceeded the failure limit.

Build Bot

Comment 8 2015-03-31 10:42:45 PDT

Created attachment 249833 [details] Archive of layout-test-results from ews103 for mac-mavericks The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews103 Port: mac-mavericks Platform: Mac OS X 10.9.5

Yoav Weiss

Comment 9 2015-03-31 14:19:53 PDT

Created attachment 249850 [details] Patch

Build Bot

Comment 10 2015-03-31 14:44:03 PDT

Comment on attachment 249850 [details] Patch Attachment 249850 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/6519012418650112 New failing tests: fast/media/media-descriptor-syntax-05.html

Build Bot

Comment 11 2015-03-31 14:44:06 PDT

Created attachment 249855 [details] Archive of layout-test-results from ews106 for mac-mavericks-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews106 Port: mac-mavericks-wk2 Platform: Mac OS X 10.9.5

Build Bot

Comment 12 2015-03-31 15:07:17 PDT

Comment on attachment 249850 [details] Patch Attachment 249850 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/5765064195833856 New failing tests: fast/media/media-descriptor-syntax-05.html

Build Bot

Comment 13 2015-03-31 15:07:22 PDT

Created attachment 249857 [details] Archive of layout-test-results from ews101 for mac-mavericks The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews101 Port: mac-mavericks Platform: Mac OS X 10.9.5

Yoav Weiss

Comment 14 2015-04-01 08:25:46 PDT

Hmm, the failing test is https://github.com/WebKit/webkit/blob/master/LayoutTests/fast/media/media-descriptor-syntax-05.html which is testing the opposite of this fix (basically making sure that this issue remains). Simon - http://www.w3.org/TR/REC-html40/types.html#h-6.13 basically tells UAs to ignore values they don't understand, which would mean that invalid MQs match by default. Did that change at a later revision of the spec?

Simon Pieters (:zcorpan)

Comment 15 2015-04-01 09:54:58 PDT

I suppose the requirement in HTML4 explains WebKit's behavior here, but that is long obsolete. https://html.spec.whatwg.org/multipage/semantics.html#attr-link-media [[ However, if the link is an external resource link, then the media attribute is prescriptive. The user agent must apply the external resource when the media attribute's value matches the environment and the other relevant conditions apply, and must not apply it otherwise. ]] -> https://html.spec.whatwg.org/multipage/infrastructure.html#matches-the-environment [[ A string matches the environment of the user if it is the empty string, a string consisting of only space characters, or is a media query list that matches the user's environment according to the definitions given in the Media Queries specification. [MQ] ]] -> https://drafts.csswg.org/mediaqueries/#error-handling [[ A media query that does not match the grammar in the previous section must be replaced by not all during parsing. ]]

Yoav Weiss

Comment 16 2015-04-01 13:36:56 PDT

Created attachment 249939 [details] Patch

Yoav Weiss

Comment 17 2015-04-15 23:34:19 PDT

ping

Brent Fulgham

Comment 18 2016-03-10 10:37:46 PST

Comment on attachment 249939 [details] Patch r=me. Please confirm tests pass before landing.

Yoav Weiss

Comment 19 2016-03-11 00:23:25 PST

Created attachment 273696 [details] Patch

Yoav Weiss

Comment 20 2016-03-11 02:23:45 PST

Comment on attachment 273696 [details] Patch Thanks for reviewing!

WebKit Commit Bot

Comment 21 2016-03-11 03:13:15 PST

Comment on attachment 273696 [details] Patch Clearing flags on attachment: 273696 Committed r198004: <http://trac.webkit.org/changeset/198004>

WebKit Commit Bot

Comment 22 2016-03-11 03:13:22 PST

All reviewed patches have been landed. Closing bug.

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

Status RESOLVED

Resolution FIXED

Priority P2

Severity Normal

Classification Unclassified

Version 528+ (Nightly build)

Hardware Unspecified

OS Unspecified

Product WebKit

Component CSS

Assignee

Nobody

Reported

2015-03-31 06:21 PDT

Modified

2016-03-11 03:13 PST History

CC List

6 users Show

URL

Keywords

Duplicates (1)

143266 View as bug list

Depends on

Blocks