Bug 106837

Summary: [CSS Shaders] Parse @-webkit-filter
Product: WebKit Reporter: Max Vujovic <mvujovic>
Component: CSSAssignee: Max Vujovic <mvujovic>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, achicu, cmarcelo, dino, gyuyoung.kim, haraken, japhet, krit, macpherson, menard, michelangelo, ojan.autocc, rakuco, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 71392    
Attachments:
Description Flags
Patch
none
Patch
eflews.bot: commit-queue-
Patch
dino: review+
Patch
mvujovic: commit-queue-
Patch none

Max Vujovic
Reported 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
Attachments
Patch (66.90 KB, patch)
2013-01-18 10:18 PST, Max Vujovic 		no flags
DetailsFormatted DiffDiff
Patch (67.02 KB, patch)
2013-01-18 11:58 PST, Max Vujovic 		eflews.bot: commit-queue-
DetailsFormatted DiffDiff
Patch (67.07 KB, patch)
2013-01-22 10:21 PST, Max Vujovic 		dino: review+
DetailsFormatted DiffDiff
Patch (67.03 KB, patch)
2013-01-28 11:10 PST, Max Vujovic 		mvujovic: commit-queue-
DetailsFormatted DiffDiff
Patch (67.01 KB, patch)
2013-01-28 11:39 PST, Max Vujovic 		no flags
DetailsFormatted DiffDiff
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.
Max Vujovic
Comment 1 2013-01-18 10:18:01 PST
Created attachment 183495 [details] Patch
Max Vujovic
Comment 2 2013-01-18 11:58:33 PST
Created attachment 183523 [details] Patch Rebased patch.
WebKit Review Bot
Comment 3 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.
EFL EWS Bot
Comment 4 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
Max Vujovic
Comment 5 2013-01-22 10:21:04 PST
Created attachment 184004 [details] Patch Attempt efl ews fix.
WebKit Review Bot
Comment 6 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.
Max Vujovic
Comment 7 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.
Max Vujovic
Comment 8 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.
Dean Jackson
Comment 9 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.
Max Vujovic
Comment 10 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.
Max Vujovic
Comment 11 2013-01-28 11:10:35 PST
Created attachment 185012 [details] Patch Addressed Dean's comments. Rebased patch. Running the bots again.
WebKit Review Bot
Comment 12 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.
Max Vujovic
Comment 13 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.
WebKit Review Bot
Comment 14 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.
Max Vujovic
Comment 15 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.
WebKit Review Bot
Comment 16 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>
WebKit Review Bot
Comment 17 2013-01-28 13:16:23 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.