Add parsing code for the the 3d-transforms parameters.
Created attachment 158647 [details] Patch
Comment on attachment 158647 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=158647&action=review > Source/WebCore/css/CSSParser.cpp:2325 > + RefPtr<CSSValue> transformValue = parseTransform(m_valueList.get()); > + if (transformValue) { > + addProperty(propId, transformValue.release(), important); Can we avoid passing a boolean? It is also not obvious from the name why we care about the comma. Anyway, I would rather avoid this. > Source/WebCore/css/CSSParser.cpp:7567 > + if (arg->unit == CSSParserValue::Function && arg->function) > + parameterValue = parseTransform(argsList, true); It looks like you are assuming that every function is a transform and then you parse it as a transform, that doesn't look quite right. What about the mat2,3,4() functions? Or texture() or array()? > Source/WebCore/css/CSSParser.cpp:7574 > + // TODO: Implement other parameters types parsing. > + // textures: https://bugs.webkit.org/show_bug.cgi?id=71442 > + // mat2, mat3, mat4: https://bugs.webkit.org/show_bug.cgi?id=71444 > + RefPtr<CSSValueList> paramValueList = CSSValueList::createSpaceSeparated(); > + while ((arg = argsList->current())) { > + // If we hit a comma it means we finished this parameter's values. The others are functions as well, how do you differ?
Comment on attachment 158647 [details] Patch Attachment 158647 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/13501825 New failing tests: css3/filters/custom/custom-filter-property-parsing.html
Created attachment 158653 [details] Archive of layout-test-results from gce-cr-linux-04 The attached test failures were seen while running run-webkit-tests on the chromium-ews. Bot: gce-cr-linux-04 Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'> Platform: Linux-2.6.39-gcg-201203291735-x86_64-with-Ubuntu-10.04-lucid
Created attachment 158672 [details] Patch
Comment on attachment 158672 [details] Patch Attachment 158672 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/13517166 New failing tests: css3/filters/custom/custom-filter-property-parsing.html
Created attachment 158680 [details] Archive of layout-test-results from gce-cr-linux-07 The attached test failures were seen while running run-webkit-tests on the chromium-ews. Bot: gce-cr-linux-07 Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'> Platform: Linux-2.6.39-gcg-201203291735-x86_64-with-Ubuntu-10.04-lucid
Created attachment 158909 [details] Patch
(In reply to comment #2) > > Source/WebCore/css/CSSParser.cpp:2325 > > + RefPtr<CSSValue> transformValue = parseTransform(m_valueList.get()); > > + if (transformValue) { > > + addProperty(propId, transformValue.release(), important); > > Can we avoid passing a boolean? It is also not obvious from the name why we care about the comma. Anyway, I would rather avoid this. Done. Present code has been slightly refactored, the parseTransform() method has been split in two to improve code reuse. > > Source/WebCore/css/CSSParser.cpp:7567 > > + if (arg->unit == CSSParserValue::Function && arg->function) > > + parameterValue = parseTransform(argsList, true); > > It looks like you are assuming that every function is a transform and then you parse it as a transform, that doesn't look quite right. What about the mat2,3,4() functions? Or texture() or array()? I've added a comment to reflect this: once we will implement the other functions (matN, array, texture), we shall branch for transform at the end of the future if-else-if block. In shall be something like this: if (arg->unit == CSSParserValue::Function && arg->function) { if (isMatN())... else if (isArray())... else if (isTexture()).... else [it might be a transform]..
Comment on attachment 158909 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=158909&action=review Patch looks good to me. Just some snippets. > Source/WebCore/css/CSSParser.cpp:7579 > // TODO: Implement other parameters types parsing. > // textures: https://bugs.webkit.org/show_bug.cgi?id=71442 > - // 3d-transforms: https://bugs.webkit.org/show_bug.cgi?id=71443 > // mat2, mat3, mat4: https://bugs.webkit.org/show_bug.cgi?id=71444 > - RefPtr<CSSValueList> paramValueList = CSSValueList::createSpaceSeparated(); > - while ((arg = argsList->current())) { > - // If we hit a comma it means we finished this parameter's values. > - if (isComma(arg)) > - break; > - if (!validUnit(arg, FNumber, CSSStrictMode)) > + // array: https://bugs.webkit.org/show_bug.cgi?id=94226 > + // 3d-transform shall be the last to be checked > + if (arg->unit == CSSParserValue::Function && arg->function) > + parameterValue = parseCustomFilterTransform(argsList); Please move the comment in to this if statement, I think you want to check all functions in the same if statement.
Created attachment 158924 [details] Patch
Comment on attachment 158924 [details] Patch Clearing flags on attachment: 158924 Committed r125845: <http://trac.webkit.org/changeset/125845>
All reviewed patches have been landed. Closing bug.
Reopening to attach new patch.
Created attachment 159022 [details] Patch
Comment on attachment 159022 [details] Patch Sorry, I might upload a patch to wrong bug.