Summary: | [Part 4] Parse the custom() function in -webkit-filter: parse the matN() functions | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Alexandru Chiculita <achicu> | ||||||
Component: | CSS | Assignee: | Alexandru Chiculita <achicu> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | allan.jensen, commit-queue, dino, esprehn+autocc, kling, krit, macpherson, menard, michelangelo, mvujovic, syoichi | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Bug Depends on: | 71446 | ||||||||
Bug Blocks: | 71395, 113695 | ||||||||
Attachments: |
|
Description
Alexandru Chiculita
2011-11-03 01:18:48 PDT
Created attachment 198969 [details]
Patch.
Patch is big because of testing.
Attachment 198969 [details] did not pass style-queue:
Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/css3/filters/custom-with-at-rule-syntax/parsing-custom-function-invalid-expected.txt', u'LayoutTests/css3/filters/custom-with-at-rule-syntax/parsing-custom-function-valid-expected.txt', u'LayoutTests/css3/filters/custom-with-at-rule-syntax/parsing-parameters-property-invalid-expected.txt', u'LayoutTests/css3/filters/custom-with-at-rule-syntax/parsing-parameters-property-valid-expected.txt', u'LayoutTests/css3/filters/custom-with-at-rule-syntax/script-tests/parsing-custom-function-invalid.js', u'LayoutTests/css3/filters/custom-with-at-rule-syntax/script-tests/parsing-custom-function-valid.js', u'LayoutTests/css3/filters/custom-with-at-rule-syntax/script-tests/parsing-parameters-property-invalid.js', u'LayoutTests/css3/filters/custom-with-at-rule-syntax/script-tests/parsing-parameters-property-valid.js', u'Source/WebCore/ChangeLog', u'Source/WebCore/GNUmakefile.list.am', u'Source/WebCore/Target.pri', u'Source/WebCore/WebCore.vcproj/WebCore.vcproj', u'Source/WebCore/WebCore.vcxproj/WebCore.vcxproj', u'Source/WebCore/WebCore.vcxproj/WebCore.vcxproj.filters', u'Source/WebCore/WebCore.xcodeproj/project.pbxproj', u'Source/WebCore/css/CSSComputedStyleDeclaration.cpp', u'Source/WebCore/css/CSSParser.cpp', u'Source/WebCore/css/CSSParser.h', u'Source/WebCore/css/CSSValue.cpp', u'Source/WebCore/css/CSSValue.h', u'Source/WebCore/css/StyleResolver.cpp', u'Source/WebCore/css/StyleResolver.h', u'Source/WebCore/css/WebKitCSSMatFunctionValue.cpp', u'Source/WebCore/css/WebKitCSSMatFunctionValue.h', u'Source/WebCore/platform/graphics/filters/CustomFilterArrayParameter.h', u'Source/WebCore/platform/graphics/filters/CustomFilterParameter.h', u'Source/WebCore/platform/graphics/filters/CustomFilterRenderer.cpp']" exit_code: 1
Source/WebCore/platform/graphics/filters/CustomFilterParameter.h:47: enum members should use InterCaps with an initial capital letter. [readability/enum_casing] [4]
Source/WebCore/css/WebKitCSSMatFunctionValue.cpp:51: An else if statement should be written as an if statement when the prior "if" concludes with a return, break, continue or goto statement. [readability/control_flow] [4]
Total errors found: 2 in 28 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 198969 [details] Patch. View in context: https://bugs.webkit.org/attachment.cgi?id=198969&action=review > Source/WebCore/ChangeLog:23 > + * GNUmakefile.list.am: Added WebKitCSSMatFunctionValue to individual build systems. > + * Target.pri: > + * WebCore.vcproj/WebCore.vcproj: > + * WebCore.vcxproj/WebCore.vcxproj: > + * WebCore.vcxproj/WebCore.vcxproj.filters: > + * WebCore.xcodeproj/project.pbxproj: > + * css/CSSComputedStyleDeclaration.cpp: > + (WebCore::valueForCustomFilterMatParameter): I really like per-function comments for significant changes. > Source/WebCore/ChangeLog:29 > + (WebCore): And hate these useless lines! :) > Source/WebCore/css/CSSParser.cpp:8483 > + // The name length must be 5; > + if (name.length() != 5) This comment adds nothing. > Source/WebCore/css/CSSParser.cpp:8499 > + UChar characterBuffer[5]; > + if (name.is8Bit()) { > + const LChar* characters8 = name.characters8(); > + for (unsigned i = 0; i < 5; ++i) > + characterBuffer[i] = characters8[i]; > + characters = characterBuffer; > + } else > + characters = name.characters16(); > + > + if (!((characters[0] == 'm' || characters[0] == 'M') > + && (characters[1] == 'a' || characters[1] == 'A') > + && (characters[2] == 't' || characters[2] == 'T') > + && characters[4] == '(')) > + return 0; Is there a reason you didn't do something like equalIgnoringCase(value->function->name, "mat2(") rather than this more complex approach? > Source/WebCore/css/WebKitCSSMatFunctionValue.cpp:2 > + * Copyright (C) 2012 Adobe Systems Incorporated. All rights reserved. It is 2013. Stop living in the past!! > Source/WebCore/css/WebKitCSSMatFunctionValue.cpp:57 > + return "mat2(" + CSSValueList::customCssText() + ')'; > + else if (length() == 9) > + return "mat3(" + CSSValueList::customCssText() + ')'; > + else if (length() == 16) > + return "mat4(" + CSSValueList::customCssText() + ')'; > + else { Use StringBuilder? Created attachment 198984 [details]
Patch for landing.
Attachment 198984 [details] did not pass style-queue:
Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/css3/filters/custom-with-at-rule-syntax/parsing-custom-function-invalid-expected.txt', u'LayoutTests/css3/filters/custom-with-at-rule-syntax/parsing-custom-function-valid-expected.txt', u'LayoutTests/css3/filters/custom-with-at-rule-syntax/parsing-parameters-property-invalid-expected.txt', u'LayoutTests/css3/filters/custom-with-at-rule-syntax/parsing-parameters-property-valid-expected.txt', u'LayoutTests/css3/filters/custom-with-at-rule-syntax/script-tests/parsing-custom-function-invalid.js', u'LayoutTests/css3/filters/custom-with-at-rule-syntax/script-tests/parsing-custom-function-valid.js', u'LayoutTests/css3/filters/custom-with-at-rule-syntax/script-tests/parsing-parameters-property-invalid.js', u'LayoutTests/css3/filters/custom-with-at-rule-syntax/script-tests/parsing-parameters-property-valid.js', u'Source/WebCore/ChangeLog', u'Source/WebCore/GNUmakefile.list.am', u'Source/WebCore/Target.pri', u'Source/WebCore/WebCore.vcproj/WebCore.vcproj', u'Source/WebCore/WebCore.vcxproj/WebCore.vcxproj', u'Source/WebCore/WebCore.vcxproj/WebCore.vcxproj.filters', u'Source/WebCore/WebCore.xcodeproj/project.pbxproj', u'Source/WebCore/css/CSSComputedStyleDeclaration.cpp', u'Source/WebCore/css/CSSParser.cpp', u'Source/WebCore/css/CSSParser.h', u'Source/WebCore/css/CSSValue.cpp', u'Source/WebCore/css/CSSValue.h', u'Source/WebCore/css/StyleResolver.cpp', u'Source/WebCore/css/StyleResolver.h', u'Source/WebCore/css/WebKitCSSMatFunctionValue.cpp', u'Source/WebCore/css/WebKitCSSMatFunctionValue.h', u'Source/WebCore/platform/graphics/filters/CustomFilterArrayParameter.h', u'Source/WebCore/platform/graphics/filters/CustomFilterParameter.h', u'Source/WebCore/platform/graphics/filters/CustomFilterRenderer.cpp']" exit_code: 1
Source/WebCore/platform/graphics/filters/CustomFilterParameter.h:47: enum members should use InterCaps with an initial capital letter. [readability/enum_casing] [4]
Total errors found: 1 in 28 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 198984 [details] Patch for landing. Clearing flags on attachment: 198984 Committed r148852: <http://trac.webkit.org/changeset/148852> All reviewed patches have been landed. Closing bug. |