Bug 183677 – Create MediaQueryParserContext to provide additional context for the evaluation of media queries

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Megan Gardner]

Create MediaQueryParserContext to provide additional context for the evaluation of media queries

[Megan Gardner]

Created attachment 335885 [details]
Patch

[Megan Gardner]

<rdar://problem/38382934>

[EWS Watchlist]

Attachment 335885 [details] did not pass style-queue:


ERROR: Source/WebCore/css/MediaList.h:47:  The parameter name "context" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WebCore/ChangeLog:8:  You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible.  [changelog/nonewtests] [5]
Total errors found: 2 in 30 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[EWS Watchlist]

Comment on attachment 335885 [details]
Patch

Attachment 335885 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/6970887

New failing tests:
css2.1/20110323/at-import-004.htm
fast/media/mq-invalid-syntax-03.html
fast/dom/css-dom-read-2.html
imported/w3c/web-platform-tests/cssom/cssimportrule.html
cssom/cssimportrule-media.html
fast/media/mq-invalid-media-feature-03.html

[EWS Watchlist]

Created attachment 335894 [details]
Archive of layout-test-results from ews101 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-sierra  Platform: Mac OS X 10.12.6

[Tim Horton]

Comment on attachment 335885 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=335885&action=review

> Source/WebCore/css/MediaList.cpp:85
> +    auto result = create(mediaString); // TODO: pass in a context if we end up needing that bit

I think we can just leave these out, and if we need to find these later we can make the argument non-optional and use the compiler.

> Source/WebCore/css/MediaQueryExpression.cpp:123
> -static inline bool featureWithZeroOrOne(const String& mediaFeature, const CSSPrimitiveValue& value)
> +static inline bool featureWithZeroOrOne(const String& mediaFeature, const CSSPrimitiveValue& value, const MediaQueryParserContext&)

Plz undo these as mentioned

> Source/WebCore/css/StyleRuleImport.cpp:79
> +    m_mediaQueries = MediaQuerySet::create(String(), MediaQueryParserContext(*document)); // TODO: not sure what string this should be created with

What’s this?!

> Source/WebCore/css/parser/MediaQueryParser.h:43
> +struct CSSParserContext;
>  class MediaQuerySet;

Sort.

[Megan Gardner]

Created attachment 335899 [details]
Patch

[Megan Gardner]

Created attachment 335904 [details]
Patch

[Tim Horton]

Comment on attachment 335904 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=335904&action=review

> Source/WebCore/css/MediaQueryParserContext.h:29
> +
> +
> +

weird extra spaces

> Source/WebCore/css/MediaQueryParserContext.h:37
> +    MediaQueryParserContext() { };

no semicolon

> Source/WebCore/css/parser/MediaQueryParser.cpp:77
> +    m_mediaQueryData.setMediaQueryParserContext(context);

Could avoid this set method if we added this to the MediaQueryData constructor and passed it in in the initializer list above. Or do other people construct MediaQueryData?

[Megan Gardner]

Comment on attachment 335904 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=335904&action=review

>> Source/WebCore/css/parser/MediaQueryParser.cpp:77
>> +    m_mediaQueryData.setMediaQueryParserContext(context);
> 
> Could avoid this set method if we added this to the MediaQueryData constructor and passed it in in the initializer list above. Or do other people construct MediaQueryData?

No, but the initializer set everything to default, and then sets things after, so I was just following what was being done for the other variables. But I guess we have this at creation time, so we can stuff it in there immediately.

[Megan Gardner]

https://trac.webkit.org/changeset/229654/webkit