Summary: | @media rules ignored in user agent style sheet html.css | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | James Craig <jcraig> | ||||||||
Component: | CSS | Assignee: | Nobody <webkit-unassigned> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | commit-queue, darin, dino, ggaren, koivisto, mitz, simon.fraser, webkit-bug-importer, zalan | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | WebKit Nightly Build | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Bug Depends on: | 169987 | ||||||||||
Bug Blocks: | 168447 | ||||||||||
Attachments: |
|
Description
James Craig
2017-03-06 18:50:59 PST
Interestingly, @media (foo) is evaluated to false, but a valid falsy media query like (inverted-colors) evaluates to true. It's because MediaQueryEvaluator::evaluate doesn't have a document or frame in the context of the default style sheet, so uses its fallback value. But this doesn't explain why the rules are applied, since the fallback value is false. Oops. The fallback is true. But amazingly, because it is created with: static NeverDestroyed<const MediaQueryEvaluator> staticScreenEval("screen"); which is cast to the MediaQueryEvaluator(bool fallback), and the "s" evaluates to true. The fun of template programming. I have a fix for the invalid creation of the MediaQueryEvaluator, but that doesn't address this bug. I'm going to have to ask Antti what he thinks is the best solution for associating a defaultStyleSheet or its rules with the current frame. There isn't a StyleResolver to use when we call RuleSet::addRulesFromSheet inside CSSDefaultStyleSheets. (In reply to Dean Jackson from comment #6) > I have a fix for the invalid creation of the MediaQueryEvaluator, but that > doesn't address this bug. > > I'm going to have to ask Antti what he thinks is the best solution for > associating a defaultStyleSheet or its rules with the current frame. Did you hear back from Antti? RuleSets for user agent stylesheets are shared between all documents. This is a major optimization that we don't want to lose. To allow media queries on UA sheets we would need to add a new type of UA sheet that is resolved per-document. Simplest approach would probably be have a different file (that is, not html4.css) for rules like this. Implementation for this should not be difficult at all. Alternatively we could move media queries later in the style pipeline. For example we could generate RuleSets for all media queries and then pick between them at style resolution time. This is a bigger refactoring. Created attachment 326094 [details]
patch
Created attachment 326117 [details]
patch
Comment on attachment 326117 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=326117&action=review > Source/WebCore/ChangeLog:9 > + To support accessibility features allow non-trivial @media rules in user agent stylesheet. need a comma after "features" > Source/WebCore/ChangeLog:48 > + Also march rules in userAgentMediaQueryStyle. march -> match > Source/WebCore/css/CSSDefaultStyleSheets.cpp:125 > + // Rulesets for these can't be global and need to be build in document context. build -> built > Source/WebCore/css/CSSDefaultStyleSheets.cpp:127 > + if (is<StyleRuleMedia>(*rule)) { I suggest using "continue" for this, just like the other if statements below. > Source/WebCore/css/CSSDefaultStyleSheets.h:50 > + static void initDefaultStyle(const Element*); I don’t understand what changed that makes this need to be public instead of private now. > I don’t understand what changed that makes this need to be public instead of
> private now.
Everything was public before this patch.
Created attachment 326126 [details]
patch
Comment on attachment 326126 [details] patch Clearing flags on attachment: 326126 Committed r224495: <https://trac.webkit.org/changeset/224495> All reviewed patches have been landed. Closing bug. |