Bug 106837 - [CSS Shaders] Parse @-webkit-filter
Summary: [CSS Shaders] Parse @-webkit-filter
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Max Vujovic
URL:
Keywords:
Depends on:
Blocks: 71392
  Show dependency treegraph
 
Reported: 2013-01-14 16:03 PST by Max Vujovic
Modified: 2013-01-28 13:16 PST (History)
14 users (show)

See Also:


Attachments
Patch (66.90 KB, patch)
2013-01-18 10:18 PST, Max Vujovic
no flags Details | Formatted Diff | Diff
Patch (67.02 KB, patch)
2013-01-18 11:58 PST, Max Vujovic
eflews.bot: commit-queue-
Details | Formatted Diff | Diff
Patch (67.07 KB, patch)
2013-01-22 10:21 PST, Max Vujovic
dino: review+
Details | Formatted Diff | Diff
Patch (67.03 KB, patch)
2013-01-28 11:10 PST, Max Vujovic
mvujovic: commit-queue-
Details | Formatted Diff | Diff
Patch (67.01 KB, patch)
2013-01-28 11:39 PST, Max Vujovic
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Max Vujovic 2013-01-14 16:03:44 PST
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
Comment 1 Max Vujovic 2013-01-18 10:18:01 PST
Created attachment 183495 [details]
Patch
Comment 2 Max Vujovic 2013-01-18 11:58:33 PST
Created attachment 183523 [details]
Patch

Rebased patch.
Comment 3 WebKit Review Bot 2013-01-18 12:02:53 PST
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.
Comment 4 EFL EWS Bot 2013-01-18 18:25:59 PST
Comment on attachment 183523 [details]
Patch

Attachment 183523 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/15942781
Comment 5 Max Vujovic 2013-01-22 10:21:04 PST
Created attachment 184004 [details]
Patch

Attempt efl ews fix.
Comment 6 WebKit Review Bot 2013-01-22 10:26:35 PST
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.
Comment 7 Max Vujovic 2013-01-22 13:33:27 PST
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.
Comment 8 Max Vujovic 2013-01-22 13:34:54 PST
Comment on attachment 184004 [details]
Patch

Setting r? efl ews passed. mac-wk2 ews appears stuck, but passed before.
Comment 9 Dean Jackson 2013-01-23 11:37:52 PST
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.
Comment 10 Max Vujovic 2013-01-23 13:22:04 PST
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.
Comment 11 Max Vujovic 2013-01-28 11:10:35 PST
Created attachment 185012 [details]
Patch

Addressed Dean's comments. Rebased patch. Running the bots again.
Comment 12 WebKit Review Bot 2013-01-28 11:15:44 PST
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.
Comment 13 Max Vujovic 2013-01-28 11:39:11 PST
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.
Comment 14 WebKit Review Bot 2013-01-28 11:43:57 PST
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.
Comment 15 Max Vujovic 2013-01-28 13:04:48 PST
Comment on attachment 185022 [details]
Patch

Setting cq+. cr-linux looks good. Mac seems stuck right now, but it passed before.
Comment 16 WebKit Review Bot 2013-01-28 13:16:16 PST
Comment on attachment 185022 [details]
Patch

Clearing flags on attachment: 185022

Committed r140997: <http://trac.webkit.org/changeset/140997>
Comment 17 WebKit Review Bot 2013-01-28 13:16:23 PST
All reviewed patches have been landed.  Closing bug.