RESOLVED FIXED 10749
All animated SVG enum properties are now ints
https://bugs.webkit.org/show_bug.cgi?id=10749
Summary All animated SVG enum properties are now ints
Eric Seidel (no email)
Reported 2006-09-06 02:28:00 PDT
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)
Attachments
Patch (402.92 KB, patch)
2011-05-18 03:34 PDT, Nikolas Zimmermann
webkit-ews: commit-queue-
Patch v2 (403.01 KB, patch)
2011-05-18 05:02 PDT, Nikolas Zimmermann
webkit.review.bot: commit-queue-
Patch v3 (404.02 KB, patch)
2011-05-18 07:09 PDT, Nikolas Zimmermann
rwlbuis: review+
webkit.review.bot: commit-queue-
Archive of layout-test-results from ec2-cr-linux-01 (1.18 MB, application/zip)
2011-05-18 08:02 PDT, WebKit Review Bot
no flags
Nikolas Zimmermann
Comment 1 2011-05-18 03:34:49 PDT
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
Comment 2 2011-05-18 03:38:02 PDT
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 3 2011-05-18 03:50:11 PDT
WebKit Review Bot
Comment 4 2011-05-18 03:56:48 PDT
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 5 2011-05-18 04:05:53 PDT
WebKit Review Bot
Comment 6 2011-05-18 04:50:13 PDT
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
Comment 7 2011-05-18 05:02:27 PDT
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 8 2011-05-18 05:24:51 PDT
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 9 2011-05-18 06:19:26 PDT
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
Comment 10 2011-05-18 07:09:20 PDT
Created attachment 93909 [details] Patch v3 Potential V8 fixes.
WebKit Review Bot
Comment 11 2011-05-18 08:01:57 PDT
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
Comment 12 2011-05-18 08:02:02 PDT
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 13 2011-05-18 08:28:04 PDT
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
Comment 14 2011-05-18 08:36:18 PDT
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.
Note You need to log in before you can comment on or make changes to this bug.