WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
164696
[SVG] Moving more special casing of SVG out of the bindings - SVGNumber/SVGPoint/SVGRect/SVGLength/SVGTransform/SVGMatrix
https://bugs.webkit.org/show_bug.cgi?id=164696
Summary
[SVG] Moving more special casing of SVG out of the bindings - SVGNumber/SVGPo...
Sam Weinig
Reported
2016-11-12 21:32:04 PST
[SVG] Moving more special casing of SVG out of the bindings - SVGPoint/SVGRect/SVGLength/SVGTransform/SVGMatrix
Attachments
Patch
(356.16 KB, patch)
2016-11-12 21:37 PST
,
Sam Weinig
no flags
Details
Formatted Diff
Diff
Patch
(354.26 KB, patch)
2016-11-12 21:59 PST
,
Sam Weinig
no flags
Details
Formatted Diff
Diff
Patch
(375.38 KB, patch)
2016-11-13 10:28 PST
,
Sam Weinig
no flags
Details
Formatted Diff
Diff
Patch
(375.40 KB, patch)
2016-11-13 10:36 PST
,
Sam Weinig
no flags
Details
Formatted Diff
Diff
Patch
(329.66 KB, patch)
2016-11-13 10:39 PST
,
Sam Weinig
no flags
Details
Formatted Diff
Diff
Patch
(335.04 KB, patch)
2016-11-13 10:41 PST
,
Sam Weinig
no flags
Details
Formatted Diff
Diff
Patch
(352.55 KB, patch)
2016-11-13 10:51 PST
,
Sam Weinig
no flags
Details
Formatted Diff
Diff
Patch
(357.14 KB, patch)
2016-11-13 11:21 PST
,
Sam Weinig
no flags
Details
Formatted Diff
Diff
Patch
(358.42 KB, patch)
2016-11-13 12:06 PST
,
Sam Weinig
no flags
Details
Formatted Diff
Diff
Patch
(357.56 KB, patch)
2016-11-13 13:04 PST
,
Sam Weinig
no flags
Details
Formatted Diff
Diff
Patch
(357.79 KB, patch)
2016-11-13 13:14 PST
,
Sam Weinig
no flags
Details
Formatted Diff
Diff
Archive of layout-test-results from ews103 for mac-yosemite
(1.28 MB, application/zip)
2016-11-13 14:14 PST
,
Build Bot
no flags
Details
Archive of layout-test-results from ews107 for mac-yosemite-wk2
(1.20 MB, application/zip)
2016-11-13 14:19 PST
,
Build Bot
no flags
Details
Archive of layout-test-results from ews117 for mac-yosemite
(1.63 MB, application/zip)
2016-11-13 14:29 PST
,
Build Bot
no flags
Details
Patch
(358.37 KB, patch)
2016-11-13 14:50 PST
,
Sam Weinig
no flags
Details
Formatted Diff
Diff
Patch
(358.83 KB, patch)
2016-11-13 16:51 PST
,
Sam Weinig
darin
: review+
Details
Formatted Diff
Diff
Show Obsolete
(15)
View All
Add attachment
proposed patch, testcase, etc.
Sam Weinig
Comment 1
2016-11-12 21:37:38 PST
Created
attachment 294650
[details]
Patch
Sam Weinig
Comment 2
2016-11-12 21:59:18 PST
Created
attachment 294651
[details]
Patch
WebKit Commit Bot
Comment 3
2016-11-12 22:02:25 PST
Attachment 294651
[details]
did not pass style-queue: ERROR: Source/WebCore/svg/SVGPolyElement.cpp:25: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WebCore/svg/SVGRect.h:22: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WebCore/svg/properties/SVGListProperty.h:156: 'newItem' is incorrectly named. It should be named 'protector' or 'protectedItem'. [readability/naming/protected] [4] ERROR: Source/WebCore/svg/properties/SVGListProperty.h:257: 'newItem' is incorrectly named. It should be named 'protector' or 'protectedItem'. [readability/naming/protected] [4] ERROR: Source/WebCore/svg/properties/SVGListProperty.h:325: 'newItem' is incorrectly named. It should be named 'protector' or 'protectedItem'. [readability/naming/protected] [4] ERROR: Source/WebCore/svg/properties/SVGListProperty.h:432: 'newItem' is incorrectly named. It should be named 'protector' or 'protectedItem'. [readability/naming/protected] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:33: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:34: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:35: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:36: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:37: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:38: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:39: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:77: The parameter name "a" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WebCore/svg/SVGLengthValue.cpp:39: Omit int when using unsigned [runtime/unsigned] [1] ERROR: Source/WebCore/svg/SVGLengthValue.cpp:44: Omit int when using unsigned [runtime/unsigned] [1] ERROR: Source/WebCore/svg/SVGLengthValue.cpp:46: Omit int when using unsigned [runtime/unsigned] [1] ERROR: Source/WebCore/svg/SVGLengthValue.cpp:50: Omit int when using unsigned [runtime/unsigned] [1] ERROR: Source/WebCore/svg/SVGLengthValue.cpp:52: Omit int when using unsigned [runtime/unsigned] [1] ERROR: Source/WebCore/svg/SVGLengthValue.cpp:53: Omit int when using unsigned [runtime/unsigned] [1] ERROR: Source/WebCore/ChangeLog:8: You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible. [changelog/nonewtests] [5] Total errors found: 21 in 117 files If any of these errors are false positives, please file a bug against check-webkit-style.
Sam Weinig
Comment 4
2016-11-13 10:28:52 PST
Created
attachment 294664
[details]
Patch
Sam Weinig
Comment 5
2016-11-13 10:36:11 PST
Created
attachment 294665
[details]
Patch
Sam Weinig
Comment 6
2016-11-13 10:39:33 PST
Created
attachment 294666
[details]
Patch
Sam Weinig
Comment 7
2016-11-13 10:41:01 PST
Created
attachment 294667
[details]
Patch
Sam Weinig
Comment 8
2016-11-13 10:51:04 PST
Created
attachment 294669
[details]
Patch
WebKit Commit Bot
Comment 9
2016-11-13 10:53:20 PST
Attachment 294669
[details]
did not pass style-queue: ERROR: Source/WebCore/svg/SVGPolyElement.cpp:25: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WebCore/svg/SVGRect.h:22: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WebCore/svg/properties/SVGListProperty.h:156: 'newItem' is incorrectly named. It should be named 'protector' or 'protectedItem'. [readability/naming/protected] [4] ERROR: Source/WebCore/svg/properties/SVGListProperty.h:257: 'newItem' is incorrectly named. It should be named 'protector' or 'protectedItem'. [readability/naming/protected] [4] ERROR: Source/WebCore/svg/properties/SVGListProperty.h:325: 'newItem' is incorrectly named. It should be named 'protector' or 'protectedItem'. [readability/naming/protected] [4] ERROR: Source/WebCore/svg/properties/SVGListProperty.h:432: 'newItem' is incorrectly named. It should be named 'protector' or 'protectedItem'. [readability/naming/protected] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:33: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:34: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:35: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:36: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:37: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:38: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:39: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:77: The parameter name "a" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WebCore/svg/SVGLengthValue.cpp:39: Omit int when using unsigned [runtime/unsigned] [1] ERROR: Source/WebCore/svg/SVGLengthValue.cpp:44: Omit int when using unsigned [runtime/unsigned] [1] ERROR: Source/WebCore/svg/SVGLengthValue.cpp:46: Omit int when using unsigned [runtime/unsigned] [1] ERROR: Source/WebCore/svg/SVGLengthValue.cpp:50: Omit int when using unsigned [runtime/unsigned] [1] ERROR: Source/WebCore/svg/SVGLengthValue.cpp:52: Omit int when using unsigned [runtime/unsigned] [1] ERROR: Source/WebCore/svg/SVGLengthValue.cpp:53: Omit int when using unsigned [runtime/unsigned] [1] ERROR: Source/WebCore/svg/SVGMatrix.h:29: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WebCore/ChangeLog:8: You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible. [changelog/nonewtests] [5] Total errors found: 22 in 118 files If any of these errors are false positives, please file a bug against check-webkit-style.
Sam Weinig
Comment 10
2016-11-13 11:21:12 PST
Created
attachment 294671
[details]
Patch
WebKit Commit Bot
Comment 11
2016-11-13 11:50:01 PST
Attachment 294671
[details]
did not pass style-queue: ERROR: Source/WebCore/svg/SVGPolyElement.cpp:25: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WebCore/svg/SVGRect.h:22: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WebCore/svg/properties/SVGListProperty.h:156: 'newItem' is incorrectly named. It should be named 'protector' or 'protectedItem'. [readability/naming/protected] [4] ERROR: Source/WebCore/svg/properties/SVGListProperty.h:257: 'newItem' is incorrectly named. It should be named 'protector' or 'protectedItem'. [readability/naming/protected] [4] ERROR: Source/WebCore/svg/properties/SVGListProperty.h:325: 'newItem' is incorrectly named. It should be named 'protector' or 'protectedItem'. [readability/naming/protected] [4] ERROR: Source/WebCore/svg/properties/SVGListProperty.h:432: 'newItem' is incorrectly named. It should be named 'protector' or 'protectedItem'. [readability/naming/protected] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:33: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:34: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:35: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:36: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:37: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:38: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:39: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:77: The parameter name "a" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WebCore/svg/SVGLengthValue.cpp:39: Omit int when using unsigned [runtime/unsigned] [1] ERROR: Source/WebCore/svg/SVGLengthValue.cpp:44: Omit int when using unsigned [runtime/unsigned] [1] ERROR: Source/WebCore/svg/SVGLengthValue.cpp:46: Omit int when using unsigned [runtime/unsigned] [1] ERROR: Source/WebCore/svg/SVGLengthValue.cpp:50: Omit int when using unsigned [runtime/unsigned] [1] ERROR: Source/WebCore/svg/SVGLengthValue.cpp:52: Omit int when using unsigned [runtime/unsigned] [1] ERROR: Source/WebCore/svg/SVGLengthValue.cpp:53: Omit int when using unsigned [runtime/unsigned] [1] ERROR: Source/WebCore/svg/SVGMatrix.h:29: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WebCore/ChangeLog:8: You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible. [changelog/nonewtests] [5] Total errors found: 22 in 118 files If any of these errors are false positives, please file a bug against check-webkit-style.
Sam Weinig
Comment 12
2016-11-13 12:06:06 PST
Created
attachment 294673
[details]
Patch
Sam Weinig
Comment 13
2016-11-13 13:04:41 PST
Created
attachment 294674
[details]
Patch
WebKit Commit Bot
Comment 14
2016-11-13 13:07:17 PST
Attachment 294674
[details]
did not pass style-queue: ERROR: Source/WebCore/svg/SVGPolyElement.cpp:25: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WebCore/svg/SVGRect.h:22: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WebCore/svg/properties/SVGListProperty.h:156: 'newItem' is incorrectly named. It should be named 'protector' or 'protectedItem'. [readability/naming/protected] [4] ERROR: Source/WebCore/svg/properties/SVGListProperty.h:257: 'newItem' is incorrectly named. It should be named 'protector' or 'protectedItem'. [readability/naming/protected] [4] ERROR: Source/WebCore/svg/properties/SVGListProperty.h:325: 'newItem' is incorrectly named. It should be named 'protector' or 'protectedItem'. [readability/naming/protected] [4] ERROR: Source/WebCore/svg/properties/SVGListProperty.h:432: 'newItem' is incorrectly named. It should be named 'protector' or 'protectedItem'. [readability/naming/protected] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:33: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:34: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:35: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:36: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:37: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:38: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:39: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:77: The parameter name "a" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WebCore/svg/SVGLengthValue.cpp:39: Omit int when using unsigned [runtime/unsigned] [1] ERROR: Source/WebCore/svg/SVGLengthValue.cpp:44: Omit int when using unsigned [runtime/unsigned] [1] ERROR: Source/WebCore/svg/SVGLengthValue.cpp:46: Omit int when using unsigned [runtime/unsigned] [1] ERROR: Source/WebCore/svg/SVGLengthValue.cpp:50: Omit int when using unsigned [runtime/unsigned] [1] ERROR: Source/WebCore/svg/SVGLengthValue.cpp:52: Omit int when using unsigned [runtime/unsigned] [1] ERROR: Source/WebCore/svg/SVGLengthValue.cpp:53: Omit int when using unsigned [runtime/unsigned] [1] ERROR: Source/WebCore/svg/SVGMatrix.h:29: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WebCore/ChangeLog:367: Need whitespace between colon and description [changelog/filechangedescriptionwhitespace] [5] ERROR: Source/WebCore/ChangeLog:369: Need whitespace between colon and description [changelog/filechangedescriptionwhitespace] [5] Total errors found: 23 in 118 files If any of these errors are false positives, please file a bug against check-webkit-style.
Sam Weinig
Comment 15
2016-11-13 13:14:22 PST
Created
attachment 294675
[details]
Patch
WebKit Commit Bot
Comment 16
2016-11-13 13:16:32 PST
Attachment 294675
[details]
did not pass style-queue: ERROR: Source/WebCore/svg/SVGPolyElement.cpp:25: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WebCore/svg/SVGRect.h:22: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WebCore/svg/properties/SVGListProperty.h:156: 'newItem' is incorrectly named. It should be named 'protector' or 'protectedItem'. [readability/naming/protected] [4] ERROR: Source/WebCore/svg/properties/SVGListProperty.h:257: 'newItem' is incorrectly named. It should be named 'protector' or 'protectedItem'. [readability/naming/protected] [4] ERROR: Source/WebCore/svg/properties/SVGListProperty.h:325: 'newItem' is incorrectly named. It should be named 'protector' or 'protectedItem'. [readability/naming/protected] [4] ERROR: Source/WebCore/svg/properties/SVGListProperty.h:432: 'newItem' is incorrectly named. It should be named 'protector' or 'protectedItem'. [readability/naming/protected] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:33: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:34: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:35: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:36: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:37: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:38: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:39: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:77: The parameter name "a" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WebCore/svg/SVGLengthValue.cpp:39: Omit int when using unsigned [runtime/unsigned] [1] ERROR: Source/WebCore/svg/SVGLengthValue.cpp:44: Omit int when using unsigned [runtime/unsigned] [1] ERROR: Source/WebCore/svg/SVGLengthValue.cpp:46: Omit int when using unsigned [runtime/unsigned] [1] ERROR: Source/WebCore/svg/SVGLengthValue.cpp:50: Omit int when using unsigned [runtime/unsigned] [1] ERROR: Source/WebCore/svg/SVGLengthValue.cpp:52: Omit int when using unsigned [runtime/unsigned] [1] ERROR: Source/WebCore/svg/SVGLengthValue.cpp:53: Omit int when using unsigned [runtime/unsigned] [1] ERROR: Source/WebCore/svg/SVGMatrix.h:29: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WebCore/ChangeLog:367: Need whitespace between colon and description [changelog/filechangedescriptionwhitespace] [5] ERROR: Source/WebCore/ChangeLog:369: Need whitespace between colon and description [changelog/filechangedescriptionwhitespace] [5] Total errors found: 23 in 118 files If any of these errors are false positives, please file a bug against check-webkit-style.
Build Bot
Comment 17
2016-11-13 14:14:35 PST
Comment on
attachment 294675
[details]
Patch
Attachment 294675
[details]
did not pass mac-ews (mac): Output:
http://webkit-queues.webkit.org/results/2510482
Number of test failures exceeded the failure limit.
Build Bot
Comment 18
2016-11-13 14:14:38 PST
Created
attachment 294676
[details]
Archive of layout-test-results from ews103 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews103 Port: mac-yosemite Platform: Mac OS X 10.10.5
Build Bot
Comment 19
2016-11-13 14:19:32 PST
Comment on
attachment 294675
[details]
Patch
Attachment 294675
[details]
did not pass mac-wk2-ews (mac-wk2): Output:
http://webkit-queues.webkit.org/results/2510491
Number of test failures exceeded the failure limit.
Build Bot
Comment 20
2016-11-13 14:19:35 PST
Created
attachment 294677
[details]
Archive of layout-test-results from ews107 for mac-yosemite-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews107 Port: mac-yosemite-wk2 Platform: Mac OS X 10.10.5
Build Bot
Comment 21
2016-11-13 14:29:16 PST
Comment on
attachment 294675
[details]
Patch
Attachment 294675
[details]
did not pass mac-debug-ews (mac): Output:
http://webkit-queues.webkit.org/results/2510498
Number of test failures exceeded the failure limit.
Build Bot
Comment 22
2016-11-13 14:29:19 PST
Created
attachment 294678
[details]
Archive of layout-test-results from ews117 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews117 Port: mac-yosemite Platform: Mac OS X 10.10.5
Sam Weinig
Comment 23
2016-11-13 14:50:35 PST
Created
attachment 294679
[details]
Patch
WebKit Commit Bot
Comment 24
2016-11-13 14:52:50 PST
Attachment 294679
[details]
did not pass style-queue: ERROR: Source/WebCore/svg/SVGPolyElement.cpp:25: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WebCore/svg/SVGRect.h:22: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WebCore/svg/properties/SVGListProperty.h:156: 'newItem' is incorrectly named. It should be named 'protector' or 'protectedItem'. [readability/naming/protected] [4] ERROR: Source/WebCore/svg/properties/SVGListProperty.h:257: 'newItem' is incorrectly named. It should be named 'protector' or 'protectedItem'. [readability/naming/protected] [4] ERROR: Source/WebCore/svg/properties/SVGListProperty.h:325: 'newItem' is incorrectly named. It should be named 'protector' or 'protectedItem'. [readability/naming/protected] [4] ERROR: Source/WebCore/svg/properties/SVGListProperty.h:432: 'newItem' is incorrectly named. It should be named 'protector' or 'protectedItem'. [readability/naming/protected] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:33: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:34: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:35: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:36: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:37: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:38: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:39: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:77: The parameter name "a" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WebCore/svg/SVGLengthValue.cpp:39: Omit int when using unsigned [runtime/unsigned] [1] ERROR: Source/WebCore/svg/SVGLengthValue.cpp:44: Omit int when using unsigned [runtime/unsigned] [1] ERROR: Source/WebCore/svg/SVGLengthValue.cpp:46: Omit int when using unsigned [runtime/unsigned] [1] ERROR: Source/WebCore/svg/SVGLengthValue.cpp:50: Omit int when using unsigned [runtime/unsigned] [1] ERROR: Source/WebCore/svg/SVGLengthValue.cpp:52: Omit int when using unsigned [runtime/unsigned] [1] ERROR: Source/WebCore/svg/SVGLengthValue.cpp:53: Omit int when using unsigned [runtime/unsigned] [1] ERROR: Source/WebCore/svg/SVGMatrix.h:29: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WebCore/ChangeLog:367: Need whitespace between colon and description [changelog/filechangedescriptionwhitespace] [5] ERROR: Source/WebCore/ChangeLog:369: Need whitespace between colon and description [changelog/filechangedescriptionwhitespace] [5] Total errors found: 23 in 119 files If any of these errors are false positives, please file a bug against check-webkit-style.
Sam Weinig
Comment 25
2016-11-13 16:51:38 PST
Created
attachment 294682
[details]
Patch
WebKit Commit Bot
Comment 26
2016-11-13 16:53:56 PST
Attachment 294682
[details]
did not pass style-queue: ERROR: Source/WebCore/svg/SVGPolyElement.cpp:25: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WebCore/svg/SVGRect.h:22: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WebCore/svg/properties/SVGListProperty.h:156: 'newItem' is incorrectly named. It should be named 'protector' or 'protectedItem'. [readability/naming/protected] [4] ERROR: Source/WebCore/svg/properties/SVGListProperty.h:257: 'newItem' is incorrectly named. It should be named 'protector' or 'protectedItem'. [readability/naming/protected] [4] ERROR: Source/WebCore/svg/properties/SVGListProperty.h:325: 'newItem' is incorrectly named. It should be named 'protector' or 'protectedItem'. [readability/naming/protected] [4] ERROR: Source/WebCore/svg/properties/SVGListProperty.h:432: 'newItem' is incorrectly named. It should be named 'protector' or 'protectedItem'. [readability/naming/protected] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:33: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:34: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:35: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:36: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:37: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:38: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:39: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/WebCore/svg/SVGTransformValue.h:77: The parameter name "a" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WebCore/svg/SVGLengthValue.cpp:39: Omit int when using unsigned [runtime/unsigned] [1] ERROR: Source/WebCore/svg/SVGLengthValue.cpp:44: Omit int when using unsigned [runtime/unsigned] [1] ERROR: Source/WebCore/svg/SVGLengthValue.cpp:46: Omit int when using unsigned [runtime/unsigned] [1] ERROR: Source/WebCore/svg/SVGLengthValue.cpp:50: Omit int when using unsigned [runtime/unsigned] [1] ERROR: Source/WebCore/svg/SVGLengthValue.cpp:52: Omit int when using unsigned [runtime/unsigned] [1] ERROR: Source/WebCore/svg/SVGLengthValue.cpp:53: Omit int when using unsigned [runtime/unsigned] [1] ERROR: Source/WebCore/svg/SVGMatrix.h:29: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WebCore/ChangeLog:367: Need whitespace between colon and description [changelog/filechangedescriptionwhitespace] [5] ERROR: Source/WebCore/ChangeLog:369: Need whitespace between colon and description [changelog/filechangedescriptionwhitespace] [5] Total errors found: 23 in 120 files If any of these errors are false positives, please file a bug against check-webkit-style.
Sam Weinig
Comment 27
2016-11-13 18:45:20 PST
Ready for review.
Darin Adler
Comment 28
2016-11-13 23:12:15 PST
Comment on
attachment 294682
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=294682&action=review
> Source/WebCore/css/SVGCSSComputedStyleDeclaration.cpp:89 > RefPtr<CSSValueList> list = CSSValueList::createCommaSeparated();
I prefer auto here. That will give us Ref rather than RefPtr.
> Source/WebCore/css/SVGCSSComputedStyleDeclaration.cpp:90 > + for (const auto& length : dashes)
I suggest just auto&; the const will happen without us explicitly asking for it.
> Source/WebCore/html/canvas/DOMPath.h:55 > + void addPath(const DOMPath* path, SVGMatrix& matrix) { addPath(path, matrix.propertyReference()); }
Should this definitely be SVGMatrix& and not const SVGMatrix&? Also strange that the first argument of these three functions is a pointer, not a reference. Finally, does this really need to be inside #if ENABLE(CANVAS_PATH)?
> Source/WebCore/rendering/style/RenderStyle.h:1755 > + void setStrokeDashArray(Vector<SVGLengthValue> array) { accessSVGStyle().setStrokeDashArray(array); }
Should take const Vector& or Vector&&, definitely not Vector, since that means it will get copied twice!
> Source/WebCore/rendering/style/SVGRenderStyle.h:81 > + static Vector<SVGLengthValue> initialStrokeDashArray() { return { }; }
Extra stray space here.
> Source/WebCore/rendering/style/SVGRenderStyle.h:218 > + void setStrokeDashArray(const Vector<SVGLengthValue>& obj)
"obj"; hmmpf, not really better than "x" or "a". Especially since this is not an "object" in any interesting sense. Used over and over again below.
> Source/WebCore/rendering/svg/SVGRenderTreeAsText.cpp:194 > + const Vector<SVGLengthValue>& dashes = svgStyle.strokeDashArray();
auto& or const auto&?
> Source/WebCore/rendering/svg/SVGRenderTreeAsText.cpp:197 > + for (const auto& length : dashes)
auto& should do, no need to emphasize const
> Source/WebCore/rendering/svg/SVGRenderTreeAsText.cpp:198 > + dashArray.append((length).value(lengthContext));
no need for parentheses around length
> Source/WebCore/svg/LinearGradientAttributes.h:46 > + SVGLengthValue x1() const { return m_x1; } > + SVGLengthValue y1() const { return m_y1; } > + SVGLengthValue x2() const { return m_x2; } > + SVGLengthValue y2() const { return m_y2; } > + > + void setX1(const SVGLengthValue& value) { m_x1 = value; m_x1Set = true; } > + void setY1(const SVGLengthValue& value) { m_y1 = value; m_y1Set = true; } > + void setX2(const SVGLengthValue& value) { m_x2 = value; m_x2Set = true; } > + void setY2(const SVGLengthValue& value) { m_y2 = value; m_y2Set = true; }
If an SVGLengthValue is efficient enough to be used as a return value then it’s efficient enough to be used as an argument. Either both should be by value or both should be const&.
> Source/WebCore/svg/PatternAttributes.h:56 > + SVGLengthValue x() const { return m_x; } > + SVGLengthValue y() const { return m_y; } > + SVGLengthValue width() const { return m_width; } > + SVGLengthValue height() const { return m_height; }
Ditto, and same below. I won’t keep commenting on it.
> Source/WebCore/svg/SVGLengthValue.cpp:39 > +static inline unsigned int storeUnit(SVGLengthMode mode, SVGLengthType type)
File uses a lot of "unsigned int" instead of "unsigned".
> Source/WebCore/svg/SVGLengthValue.cpp:47 > + unsigned int mode = unit >> 4; > + return static_cast<SVGLengthMode>(mode);
No need for a local variable for this.
> Source/WebCore/svg/SVGLengthValue.cpp:54 > + unsigned int mode = unit >> 4; > + unsigned int type = unit ^ (mode << 4); > + return static_cast<SVGLengthType>(type);
This is a strange way to mask off high bits, shifting twice and using exclusive or. I would instead write: return static_cast<SVGLengthType>(unit & ((1 << 4) - 1));
> Source/WebCore/svg/SVGLengthValue.cpp:123 > + : m_valueInSpecifiedUnits(0)
Seems like this could be initialized in the class definition instead.
> Source/WebCore/svg/SVGLengthValue.cpp:276 > + SVGLengthType svgType;
I would call this just "type".
> Source/WebCore/svg/SVGLengthValue.cpp:315 > + case CSSPrimitiveValue::CSS_UNKNOWN: > + default: > + svgType = LengthTypeUnknown; > + break; > + }; > + > + if (svgType == LengthTypeUnknown) > + return { };
Seems like we should just return from inside the switch instead.
> Source/WebCore/svg/SVGMatrixValue.h:2 > + * Copyright (C) Research In Motion Limited 2010. All rights reserved.
Looks like you rewrote almost this entire file, but left a different copyright and license on it.
> Source/WebCore/svg/SVGMatrixValue.h:34 > + SVGMatrixValue() { } > + SVGMatrixValue(const AffineTransform& other) > + : AffineTransform(other) > + { > + }
= default for these two?
> Source/WebCore/svg/SVGSVGElement.cpp:255 > + SVGLengthValue length = SVGLengthValue::construct(LengthModeWidth, value, parseError, ForbidNegativeLengths);
auto would be nice here
> Source/WebCore/svg/SVGSVGElement.cpp:259 > + length = SVGLengthValue(LengthModeWidth, ASCIILiteral("100%"));
maybe use initializer list syntax? length = { LengthModeWidth, String { ASCIILiteral { "100%" } } }; Or maybe not.
> Source/WebCore/svg/SVGSVGElement.cpp:263 > + SVGLengthValue length = SVGLengthValue::construct(LengthModeHeight, value, parseError, ForbidNegativeLengths);
Ditto.
> Source/WebCore/svg/SVGSVGElement.cpp:267 > + length = SVGLengthValue(LengthModeHeight, ASCIILiteral("100%"));
Ditto.
> Source/WebCore/svg/SVGSVGElement.h:95 > + Ref<NodeList> getIntersectionList(SVGRect&, SVGElement* referenceElement); > + Ref<NodeList> getEnclosureList(SVGRect&, SVGElement* referenceElement); > + static bool checkIntersection(const SVGElement*, SVGRect&); > + static bool checkEnclosure(const SVGElement*, SVGRect&);
Can some of these be const SVGRect& instead?
> Source/WebCore/svg/SVGSVGElement.h:105 > + static Ref<SVGTransform> createSVGTransformFromMatrix(SVGMatrix&);
Can this be const SVGMatrix& like before?
> Source/WebCore/svg/SVGTransformValue.cpp:36 > + : m_type(SVG_TRANSFORM_UNKNOWN) > + , m_angle(0)
Could we initialize these in the class definition?
> Source/WebCore/svg/SVGTransformValue.cpp:45 > + m_matrix = AffineTransform(0, 0, 0, 0, 0, 0);
Is there a nicer way to write this?
> Source/WebCore/svg/SVGTransformValue.cpp:210 > + double angleInRad = deg2rad(m_angle); > + double cosAngle = cos(angleInRad); > + double sinAngle = sin(angleInRad);
Should use std:: versions because they are properly overloaded, although I suppose here we want to use double.
> Source/WebCore/svg/SVGTransformValue.cpp:212 > + float cx = narrowPrecisionToFloat(cosAngle != 1 ? (m_matrix.e() * (1 - cosAngle) - m_matrix.f() * sinAngle) / (1 - cosAngle) / 2 : 0); > + float cy = narrowPrecisionToFloat(cosAngle != 1 ? (m_matrix.e() * sinAngle / (1 - cosAngle) + m_matrix.f()) / 2 : 0);
Why narrow to float?
> Source/WebCore/svg/SVGTransformValue.h:67 > + // Internal use only (animation system)
Not sure this comment is all that great.
> Source/WebCore/svg/SVGTransformValue.h:77 > + friend bool operator==(const SVGTransformValue& a, const SVGTransformValue& b);
No need for argument names.
> Source/WebCore/svg/properties/SVGTransformListPropertyTearOff.h:62 > + RefPtr<SVGTransform> wrapper = m_values->consolidate();
auto?
Sam Weinig
Comment 29
2016-11-14 13:21:50 PST
Committed
r208705
: <
http://trac.webkit.org/changeset/208705
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug