Bug 10749 – All animated SVG enum properties are now ints

• Component: SVG
• Status: RESOLVED FIXED

[Eric Seidel (no email)]

One of the side effects of fixing bug 10490 is that all of the previously SVGAnimatedEnumeration types are now ints.  This should be fixed.  All enums should be stored on the classes with the proper enum types to allow for proper type-checking.

One of many examples:

ANIMATED_PROPERTY_DECLARATIONS(int, int, XChannelSelector, xChannelSelector)

[Nikolas Zimmermann]

Created attachment 93895 [details]
Patch

It just took 5 years but here's the fix... it's extra-large because of dozens of new testcases and pixel test results. Don't be afraid! :-)

[WebKit Review Bot]

Attachment 93895 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/plat..." exit_code: 1

Source/WebCore/svg/SVGAnimatedEnumeration.h:25:  Alphabetical sorting problem.  [build/include_order] [4]
Source/WebCore/svg/SVGFEBlendElement.cpp:54:  Tab found; better to use spaces  [whitespace/tab] [1]
Total errors found: 2 in 118 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Early Warning System Bot]

Comment on attachment 93895 [details]
Patch

Attachment 93895 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/8709453

[WebKit Review Bot]

Comment on attachment 93895 [details]
Patch

Attachment 93895 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/8709450

[Collabora GTK+ EWS bot]

Comment on attachment 93895 [details]
Patch

Attachment 93895 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/8702712

[WebKit Review Bot]

Comment on attachment 93895 [details]
Patch

Attachment 93895 [details] did not pass cr-mac-ews (chromium):
Output: http://queues.webkit.org/results/8702718

[Nikolas Zimmermann]

Created attachment 93899 [details]
Patch v2

Should build everywhere except for cr-mac/linux, as the V8 build remains broken.
I probably need to generate V8SVGMarkerElement locally to spot the problem...

[WebKit Review Bot]

Comment on attachment 93899 [details]
Patch v2

Attachment 93899 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/8705659

[WebKit Review Bot]

Comment on attachment 93899 [details]
Patch v2

Attachment 93899 [details] did not pass cr-mac-ews (chromium):
Output: http://queues.webkit.org/results/8707476

[Nikolas Zimmermann]

Created attachment 93909 [details]
Patch v3

Potential V8 fixes.

[WebKit Review Bot]

Comment on attachment 93909 [details]
Patch v3

Attachment 93909 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/8705689

New failing tests:
svg/filters/feComponentTransfer-style-crash.xhtml
svg/filters/feBlend-invalid-mode.xhtml
svg/filters/feDisplacementMap-crash-test.xhtml

[WebKit Review Bot]

Created attachment 93916 [details]
Archive of layout-test-results from ec2-cr-linux-01

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: ec2-cr-linux-01  Port: Chromium  Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick

[Rob Buis]

Comment on attachment 93909 [details]
Patch v3

View in context: https://bugs.webkit.org/attachment.cgi?id=93909&action=review

Looks good, r=me.

> Source/WebCore/ChangeLog:23
> +

I think we came up with a better comment on IRC:

alert(myClipPath.getAttribute('clipPathUnits')); <- without this patch it says "1", now it says "userSpaceOnUse" as expected, and as other browsers do.

[Nikolas Zimmermann]

Landed in r86765. Unfortunately I need to leave now, Rob said he's gonna watch the bots if anything breaks. We probably need some rebaselines.