This will just add the parsing of the property (no CSSStyleSelector code yet)
Created attachment 113493 [details] Patch V1
Where is the spec for this? Would help to have it to compare against when reviewing the code.
(In reply to comment #2) > Where is the spec for this? Would help to have it to compare against when reviewing the code. The shaders parsing is described here: https://dvcs.w3.org/hg/FXTF/raw-file/tip/custom/index.html It falls into the CSS Filters code: https://dvcs.w3.org/hg/FXTF/raw-file/tip/filters/publish/Filters.html
Comment on attachment 113493 [details] Patch V1 View in context: https://bugs.webkit.org/attachment.cgi?id=113493&action=review Cool. I think there are just minor fixes. The main thing is making sure the file is included in the platform build systems. > Source/WebCore/css/CSSParser.cpp:6475 > +static bool skipCommaValue(CSSParserValueList* argsList) > +{ This is not an issue on the patch, but it makes me realise that we're being inconsistent with filters expecting space-separated values and shaders expecting comma-separated. > Source/WebCore/css/CSSParser.cpp:6511 > + // vertexMesh: +<integer>{1,2}[wsp<box>][wspâdetached'] > + // > + // param-value: true|false[wsp+true|false]{0-3} | > + // <number>[wsp+<number>]{0-3} | > + // <array> | > + // <transform> | > + // <texture(<uri>)> > + // array: âarray(â<number>[wsp<number>]*â)â > + // css-3d-transform: <transform-function>;[<transform-function>]* > + // transform: <css-3d-transform> | <mat> > + // mat: âmat2(â<number>(,<number>){3}â)â | > + // âmat3(â<number>(,<number>){8}â)â | > + // âmat4(â<number>(,<number>){15}â)â ) Unicode cruft here. > Source/WebCore/css/CSSParser.cpp:6537 > + if (!shadersList->length() || !hadAtLeastOneCustomShader || shadersList->length() > 2 || !skipCommaValue(argsList)) I wonder if we can come up with a better name than skipCommaValue, since it's returning a boolean? This isn't a big deal. > Source/WebCore/css/CSSParser.cpp:6550 > + int intValue = static_cast<int>(arg->fValue); > + meshSizeList->append(primitiveValueCache()->createValue(intValue, CSSPrimitiveValue::CSS_NUMBER)); No need for the variable - just use it in createValue. > Source/WebCore/css/CSSParser.cpp:6588 > + // TODO: Implement other parameters types parsing. Could we add a bugzilla link here? > Source/WebCore/css/CSSParser.cpp:6591 > + // If we hit a comma it means we finished this parameters values. Super-nit "parameter's" :) > Source/WebCore/css/CSSParser.h:191 > +#if ENABLE(CSS_SHADERS) > + PassRefPtr<WebKitCSSFilterValue> parseCustomFilter(CSSParserValue*); > +#endif Could we move this to below parseFilter()? > Source/WebCore/css/CSSValueKeywords.in:856 > +#if defined(ENABLE_CSS_SHADERS) && ENABLE_CSS_SHADERS > +// -webkit-filter > +// values for the custom() function > +//border-box > +//padding-box > +//content-box Guard this with CSS_FILTERS as well > Source/WebCore/css/WebKitCSSShaderValue.h:1 > +/* I don't see this file being added to any of the build systems. You'll need to edit the Xcode project, the VS file, the gyp stuff, etc. > Source/WebCore/css/WebKitCSSShaderValue.h:32 > + We typically put #if ENABLE(CSS_SHADERS) in these files too.
Created attachment 113650 [details] Patch V2 Thanks for the comments, I've updated the patch.
Comment on attachment 113650 [details] Patch V2 View in context: https://bugs.webkit.org/attachment.cgi?id=113650&action=review All good, as long as you fix the tests. Maybe have JS extract the url into just the last component? > LayoutTests/css3/filters/custom-filter-property-parsing-expected.txt:10 > +PASS declaration.getPropertyValue('-webkit-filter') is 'custom(url(file:///Volumes/SSD/Users/achicu/code/webkit/git/webkit/LayoutTests/css3/filters/vertex.shader))' This is going to break for everyone but you :) > LayoutTests/css3/filters/custom-filter-property-parsing-expected.txt:16 > +PASS subRule.cssText is 'custom(url(file:///Volumes/SSD/Users/achicu/code/webkit/git/webkit/LayoutTests/css3/filters/vertex.shader))' ditto > LayoutTests/css3/filters/custom-filter-property-parsing-expected.txt:27 > +PASS subRule.cssText is 'custom(none url(file:///Volumes/SSD/Users/achicu/code/webkit/git/webkit/LayoutTests/css3/filters/fragment.shader))' ditto (and a couple more) > Source/WebCore/css/CSSParser.cpp:6488 > + // the implementation of the syntax: Typically we don't put instructions like this, because it is obvious to anyone editing the code.
Comment on attachment 113650 [details] Patch V2 View in context: https://bugs.webkit.org/attachment.cgi?id=113650&action=review > Source/WebCore/css/CSSParser.cpp:6487 > + // Listing the format for the custom filter here. Please update this when changing Typically we don't put instructions like this, because it is obvious to anyone editing the code.
Created attachment 114068 [details] Patch for landing
Comment on attachment 114068 [details] Patch for landing Rejecting attachment 114068 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2 Last 500 characters of output: 3/filters/custom-filter-property-parsing-invalid-expected.txt patching file LayoutTests/css3/filters/custom-filter-property-parsing-invalid.html patching file LayoutTests/css3/filters/custom-filter-property-parsing.html patching file LayoutTests/css3/filters/script-tests/custom-filter-property-parsing-invalid.js patching file LayoutTests/css3/filters/script-tests/custom-filter-property-parsing.js Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--force']" exit_code: 1 Full output: http://queues.webkit.org/results/10378078
I tried to land this patch manually but noticed a couple of build errors (they were on mac, that's why EWS didn't catch them - I guess you're on Windows). Source/WebCore/css/CSSValue.cpp:90:13: error: enumeration value 'WebKitCSSShaderClass' not handled in switch [-Werror,-Wswitch-enum,3] Source/WebCore/css/CSSValue.cpp:165:13: error: enumeration value 'WebKitCSSShaderClass' not handled in switch [-Werror,-Wswitch-enum,3] Source/WebCore/css/WebKitCSSShaderValue.h:39:7: error: 'WebCore::WebKitCSSShaderValue' has virtual functions but non-virtual destructor [-Werror,-Wnon-virtual-dtor,3] All simple fixes thankfully.
(In reply to comment #10) > I tried to land this patch manually but noticed a couple of build errors (they were on mac, that's why EWS didn't catch them - I guess you're on Windows). > > Source/WebCore/css/CSSValue.cpp:90:13: error: enumeration value 'WebKitCSSShaderClass' not handled in switch [-Werror,-Wswitch-enum,3] > Source/WebCore/css/CSSValue.cpp:165:13: error: enumeration value 'WebKitCSSShaderClass' not handled in switch [-Werror,-Wswitch-enum,3] > > Source/WebCore/css/WebKitCSSShaderValue.h:39:7: error: 'WebCore::WebKitCSSShaderValue' has virtual functions but non-virtual destructor [-Werror,-Wnon-virtual-dtor,3] > > All simple fixes thankfully. Yes, should be simple. I'm actually using Mac, just that the CSSValue.cpp file was heavily changed after I've posted the patch: http://trac.webkit.org/log/trunk/Source/WebCore/css/CSSValue.cpp I will update the patch and post for commit again.
Created attachment 114248 [details] Patch for landing
Comment on attachment 114248 [details] Patch for landing Clearing flags on attachment: 114248 Committed r99695: <http://trac.webkit.org/changeset/99695>
All reviewed patches have been landed. Closing bug.
<rdar://problem/10420439>