| Summary: | Invalid media feature evaluates to true | ||
|---|---|---|---|
| Product: | WebKit | Reporter: | Simon Pieters (:zcorpan) <zcorpan> |
| Component: | CSS | Assignee: | Nobody <webkit-unassigned> |
| Status: | RESOLVED FIXED | ||
| Severity: | Normal | CC: | benjamin, buildbot, cdumez, commit-queue, rniwa, yoav |
| Priority: | P2 | ||
| Version: | 528+ (Nightly build) | ||
| Hardware: | Unspecified | ||
| OS: | Unspecified | ||
| Attachments: | |||
|
Description
Simon Pieters (:zcorpan)
2015-03-31 06:21:16 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 *** Bug 143266 has been marked as a duplicate of this bug. *** Created attachment 249827 [details]
Patch
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. 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. 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
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. 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
Created attachment 249850 [details]
Patch
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 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
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 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
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? 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. ]] Created attachment 249939 [details]
Patch
ping Comment on attachment 249939 [details]
Patch
r=me. Please confirm tests pass before landing.
Created attachment 273696 [details]
Patch
Comment on attachment 273696 [details]
Patch
Thanks for reviewing!
Comment on attachment 273696 [details] Patch Clearing flags on attachment: 273696 Committed r198004: <http://trac.webkit.org/changeset/198004> All reviewed patches have been landed. Closing bug. |