Bug 10749 - All animated SVG enum properties are now ints
Summary: All animated SVG enum properties are now ints
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: 420+
Hardware: Macintosh OS X 10.4
: P4 Normal
Assignee: Nikolas Zimmermann
URL:
Keywords:
Depends on:
Blocks: 41761
  Show dependency treegraph
 
Reported: 2006-09-06 02:28 PDT by Eric Seidel (no email)
Modified: 2011-05-18 08:36 PDT (History)
6 users (show)

See Also:


Attachments
Patch (402.92 KB, patch)
2011-05-18 03:34 PDT, Nikolas Zimmermann
webkit-ews: commit-queue-
Details | Formatted Diff | Diff
Patch v2 (403.01 KB, patch)
2011-05-18 05:02 PDT, Nikolas Zimmermann
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff
Patch v3 (404.02 KB, patch)
2011-05-18 07:09 PDT, Nikolas Zimmermann
rwlbuis: review+
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff
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 Details

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Seidel (no email) 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)
Comment 1 Nikolas Zimmermann 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! :-)
Comment 2 WebKit Review Bot 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.
Comment 3 Early Warning System Bot 2011-05-18 03:50:11 PDT
Comment on attachment 93895 [details]
Patch

Attachment 93895 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/8709453
Comment 4 WebKit Review Bot 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
Comment 5 Collabora GTK+ EWS bot 2011-05-18 04:05:53 PDT
Comment on attachment 93895 [details]
Patch

Attachment 93895 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/8702712
Comment 6 WebKit Review Bot 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
Comment 7 Nikolas Zimmermann 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...
Comment 8 WebKit Review Bot 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
Comment 9 WebKit Review Bot 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
Comment 10 Nikolas Zimmermann 2011-05-18 07:09:20 PDT
Created attachment 93909 [details]
Patch v3

Potential V8 fixes.
Comment 11 WebKit Review Bot 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
Comment 12 WebKit Review Bot 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
Comment 13 Rob Buis 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.
Comment 14 Nikolas Zimmermann 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.