Bug 106837 – [CSS Shaders] Parse @-webkit-filter

• Component: CSS
• Status: RESOLVED FIXED

[Max Vujovic]

The new CSS Custom Filters syntax includes an @filter rule:
@filter IDENT { <custom-filter-description> }

This bug covers parsing the at-rule (not its internal properties).

Spec: https://dvcs.w3.org/hg/FXTF/raw-file/tip/filters/index.html#the-atfilter-rule

[Max Vujovic]

Created attachment 183495 [details]
Patch

[Max Vujovic]

Created attachment 183523 [details]
Patch

Rebased patch.

[WebKit Review Bot]

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

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/css3..." exit_code: 1
Source/WebCore/css/CSSRule.h:63:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebCore/css/CSSPropertySourceData.h:101:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebCore/css/CSSPropertySourceData.h:103:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Total errors found: 3 in 36 files


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

[EFL EWS Bot]

Comment on attachment 183523 [details]
Patch

Attachment 183523 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/15942781

[Max Vujovic]

Created attachment 184004 [details]
Patch

Attempt efl ews fix.

[WebKit Review Bot]

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

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/css3..." exit_code: 1
Source/WebCore/css/CSSRule.h:63:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebCore/css/CSSPropertySourceData.h:101:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebCore/css/CSSPropertySourceData.h:103:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Total errors found: 3 in 36 files


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

[Max Vujovic]

Comment on attachment 184004 [details]
Patch

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

>> Source/WebCore/css/CSSPropertySourceData.h:101
>> +        SUPPORTS_RULE,
> 
> enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]

The style errors are unavoidable in order to match the local style of the enums.

[Max Vujovic]

Comment on attachment 184004 [details]
Patch

Setting r? efl ews passed. mac-wk2 ews appears stuck, but passed before.

[Dean Jackson]

Comment on attachment 184004 [details]
Patch

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

> LayoutTests/css3/filters/custom/custom-filter-parsing-at-rule-invalid-expected.txt:46
> +Unprefixed rule.
> +@filter my-filter { }
> +PASS insertRuleException instanceof DOMException is true
> +PASS insertRuleException.code is 12
> +PASS successfullyParsed is true

You already have this test.

> LayoutTests/css3/filters/script-tests/custom-filter-parsing-at-rule-invalid.js:20
> +    try {
> +    	exception = null;
> +	    stylesheet.insertRule(rule, 0);
> +	    testFailed("\"" + rule + "\" did not throw a syntax error.");
> +    } catch (e) {
> +    	insertRuleException = e;
> +    	shouldBeTrue("insertRuleException instanceof DOMException");
> +    	shouldEvaluateTo("insertRuleException.code", DOMException.SYNTAX_ERR)
> +    }

Nit: indentation all weird :)

> LayoutTests/css3/filters/script-tests/custom-filter-parsing-at-rule-invalid.js:33
> +testInvalidFilterAtRule("Unprefixed rule.", "@filter my-filter { }");

Here is the dupe.

> Source/WebCore/ChangeLog:41
> +        (WebCore):

Remove this line.

> Source/WebCore/ChangeLog:57
> +        (WebCore):

Ditto.

> Source/WebCore/ChangeLog:66
> +        (WebCore):

Ditto.

> Source/WebCore/ChangeLog:76
> +        (WebCore):

Ditto.

> Source/WebCore/ChangeLog:87
> +        (WebCore):

Ditto.

[Max Vujovic]

Comment on attachment 184004 [details]
Patch

Thanks for the review, Dean!

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

>> LayoutTests/css3/filters/custom/custom-filter-parsing-at-rule-invalid-expected.txt:46
>> +PASS successfullyParsed is true
> 
> You already have this test.

Oops! Thanks for catching that.

>> LayoutTests/css3/filters/script-tests/custom-filter-parsing-at-rule-invalid.js:20
>> +    }
> 
> Nit: indentation all weird :)

Indeed. Will fix.

>> Source/WebCore/ChangeLog:41
>> +        (WebCore):
> 
> Remove this line.

Will do.

[Max Vujovic]

Created attachment 185012 [details]
Patch

Addressed Dean's comments. Rebased patch. Running the bots again.

[WebKit Review Bot]

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

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/css3/filters/custom/custom-filter-parsing-at-rule-invalid-expected.txt', u'LayoutTests/css3/filters/custom/custom-filter-parsing-at-rule-invalid.html', u'LayoutTests/css3/filters/custom/custom-filter-parsing-at-rule-valid-expected.txt', u'LayoutTests/css3/filters/custom/custom-filter-parsing-at-rule-valid.html', u'LayoutTests/css3/filters/custom/custom-filter-property-parsing-invalid.html', u'LayoutTests/css3/filters/custom/custom-filter-property-parsing.html', u'LayoutTests/css3/filters/script-tests/custom-filter-parsing-at-rule-invalid.js', u'LayoutTests/css3/filters/script-tests/custom-filter-parsing-at-rule-valid.js', u'LayoutTests/css3/filters/script-tests/custom-filter-parsing-common.js', u'LayoutTests/css3/filters/script-tests/custom-filter-property-parsing-invalid.js', u'LayoutTests/css3/filters/script-tests/custom-filter-property-parsing.js', u'LayoutTests/platform/chromium/css3/filters/custom/custom-filter-parsing-at-rule-valid-expected.txt', u'Source/WebCore/CMakeLists.txt', u'Source/WebCore/ChangeLog', u'Source/WebCore/DerivedSources.cpp', u'Source/WebCore/DerivedSources.make', u'Source/WebCore/DerivedSources.pri', u'Source/WebCore/GNUmakefile.list.am', u'Source/WebCore/Target.pri', u'Source/WebCore/WebCore.gypi', u'Source/WebCore/WebCore.xcodeproj/project.pbxproj', u'Source/WebCore/bindings/js/JSCSSRuleCustom.cpp', u'Source/WebCore/bindings/objc/DOMCSS.mm', u'Source/WebCore/bindings/v8/custom/V8CSSRuleCustom.cpp', u'Source/WebCore/css/CSSGrammar.y.in', u'Source/WebCore/css/CSSParser.cpp', u'Source/WebCore/css/CSSParser.h', u'Source/WebCore/css/CSSPropertySourceData.h', u'Source/WebCore/css/CSSRule.h', u'Source/WebCore/css/CSSRule.idl', u'Source/WebCore/css/StyleRule.cpp', u'Source/WebCore/css/StyleRule.h', u'Source/WebCore/css/StyleSheetContents.cpp', u'Source/WebCore/css/WebKitCSSFilterRule.cpp', u'Source/WebCore/css/WebKitCSSFilterRule.h', u'Source/WebCore/css/WebKitCSSFilterRule.idl']" exit_code: 1
Source/WebCore/css/CSSRule.h:63:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebCore/css/CSSPropertySourceData.h:101:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebCore/css/CSSPropertySourceData.h:103:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Total errors found: 3 in 36 files


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

[Max Vujovic]

Created attachment 185022 [details]
Patch

Noticed that there was an unnecessary line in a JavaScript test, and removed it. The line was "exception = null" in custom-filter-parsing-common.js.

[WebKit Review Bot]

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

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/css3/filters/custom/custom-filter-parsing-at-rule-invalid-expected.txt', u'LayoutTests/css3/filters/custom/custom-filter-parsing-at-rule-invalid.html', u'LayoutTests/css3/filters/custom/custom-filter-parsing-at-rule-valid-expected.txt', u'LayoutTests/css3/filters/custom/custom-filter-parsing-at-rule-valid.html', u'LayoutTests/css3/filters/custom/custom-filter-property-parsing-invalid.html', u'LayoutTests/css3/filters/custom/custom-filter-property-parsing.html', u'LayoutTests/css3/filters/script-tests/custom-filter-parsing-at-rule-invalid.js', u'LayoutTests/css3/filters/script-tests/custom-filter-parsing-at-rule-valid.js', u'LayoutTests/css3/filters/script-tests/custom-filter-parsing-common.js', u'LayoutTests/css3/filters/script-tests/custom-filter-property-parsing-invalid.js', u'LayoutTests/css3/filters/script-tests/custom-filter-property-parsing.js', u'LayoutTests/platform/chromium/css3/filters/custom/custom-filter-parsing-at-rule-valid-expected.txt', u'Source/WebCore/CMakeLists.txt', u'Source/WebCore/ChangeLog', u'Source/WebCore/DerivedSources.cpp', u'Source/WebCore/DerivedSources.make', u'Source/WebCore/DerivedSources.pri', u'Source/WebCore/GNUmakefile.list.am', u'Source/WebCore/Target.pri', u'Source/WebCore/WebCore.gypi', u'Source/WebCore/WebCore.xcodeproj/project.pbxproj', u'Source/WebCore/bindings/js/JSCSSRuleCustom.cpp', u'Source/WebCore/bindings/objc/DOMCSS.mm', u'Source/WebCore/bindings/v8/custom/V8CSSRuleCustom.cpp', u'Source/WebCore/css/CSSGrammar.y.in', u'Source/WebCore/css/CSSParser.cpp', u'Source/WebCore/css/CSSParser.h', u'Source/WebCore/css/CSSPropertySourceData.h', u'Source/WebCore/css/CSSRule.h', u'Source/WebCore/css/CSSRule.idl', u'Source/WebCore/css/StyleRule.cpp', u'Source/WebCore/css/StyleRule.h', u'Source/WebCore/css/StyleSheetContents.cpp', u'Source/WebCore/css/WebKitCSSFilterRule.cpp', u'Source/WebCore/css/WebKitCSSFilterRule.h', u'Source/WebCore/css/WebKitCSSFilterRule.idl']" exit_code: 1
Source/WebCore/css/CSSRule.h:63:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebCore/css/CSSPropertySourceData.h:101:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebCore/css/CSSPropertySourceData.h:103:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Total errors found: 3 in 36 files


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

[Max Vujovic]

Comment on attachment 185022 [details]
Patch

Setting cq+. cr-linux looks good. Mac seems stuck right now, but it passed before.

[WebKit Review Bot]

Comment on attachment 185022 [details]
Patch

Clearing flags on attachment: 185022

Committed r140997: <http://trac.webkit.org/changeset/140997>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.