Bug 20430 - DOMWindow is missing lots of SVG* constructor objects
Summary: DOMWindow is missing lots of SVG* constructor objects
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC All
: P2 Normal
Assignee: Nikolas Zimmermann
URL:
Keywords:
: 21806 22940 (view as bug list)
Depends on: 21846
Blocks: 15494 20434 27243
  Show dependency treegraph
 
Reported: 2008-08-18 14:14 PDT by Holger Freyther
Modified: 2009-12-25 13:04 PST (History)
7 users (show)

See Also:


Attachments
Bind SVGElementConstructor as SVGElement (2.65 KB, patch)
2008-08-18 14:17 PDT, Holger Freyther
darin: review-
Details | Formatted Diff | Diff
Expose many missing constructors on window (41.68 KB, patch)
2008-10-22 14:13 PDT, Eric Seidel (no email)
sam: review-
Details | Formatted Diff | Diff
list of idls w/o GenerateConstructor (4.63 KB, text/plain)
2009-07-15 14:19 PDT, Eric Seidel (no email)
no flags Details
Updated patch (93.37 KB, patch)
2009-12-24 16:31 PST, Nikolas Zimmermann
no flags Details | Formatted Diff | Diff
Updated patch v2 (94.43 KB, patch)
2009-12-24 17:01 PST, Nikolas Zimmermann
no flags Details | Formatted Diff | Diff
Updated patch v3 (95.09 KB, patch)
2009-12-24 17:37 PST, Nikolas Zimmermann
krit: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Holger Freyther 2008-08-18 14:14:51 PDT
SVGElement and SVGElement.prototype are not available from within JavaScript. According to http://www.w3.org/TR/SVG11/ecmascript-binding.html (search Object SVGElement) and what firefox presents it should be available.
Comment 1 Holger Freyther 2008-08-18 14:17:18 PDT
Created attachment 22858 [details]
Bind SVGElementConstructor as SVGElement

Add a patch, test case and test result.
Comment 2 Mark Rowe (bdash) 2008-08-19 21:41:54 PDT
I would expect more test results to need updated.  We have at least one test (LayoutTests/fast/dom/Window/window-properties.html) that dumps all properties of the window object.
Comment 3 Mark Rowe (bdash) 2008-08-19 21:42:59 PDT
I should also add that since we have that test, I don't think we need a specific test for the presence of SVGElement and its prototype.
Comment 4 Mark Rowe (bdash) 2008-08-19 21:43:29 PDT
Comment on attachment 22858 [details]
Bind SVGElementConstructor as SVGElement

r=me, if you add a ChangeLog entry, and update the layout test results.
Comment 5 Darin Adler 2008-10-12 17:04:00 PDT
Comment on attachment 22858 [details]
Bind SVGElementConstructor as SVGElement

Since this has been sitting around for two months, setting the review flag to review- since this doesn't have ChangeLog and updated regression test results yet.
Comment 6 Eric Seidel (no email) 2008-10-21 16:28:38 PDT
It is not clear from http://www.w3.org/TR/SVG11/ecmascript-binding.html that "window.SVGElement" needs to exist.  But I'm not sure I'm against it doing so.  I'm not really sure why window.HTMLElement exists.  I guess w/o it you can't say "foo instanceof SVGElement"?
Comment 7 Eric Seidel (no email) 2008-10-22 14:11:27 PDT
*** Bug 21806 has been marked as a duplicate of this bug. ***
Comment 8 Eric Seidel (no email) 2008-10-22 14:13:12 PDT
Created attachment 24572 [details]
Expose many missing constructors on window

 LayoutTests/ChangeLog                              |   10 ++
 .../svg/custom/global-constructors-expected.txt    |  166 ++++++++++++++++++++
 LayoutTests/svg/custom/global-constructors.html    |   13 ++
 WebCore/ChangeLog                                  |   38 +++++
 WebCore/DerivedSources.make                        |    1 +
 WebCore/WebCore.xcodeproj/project.pbxproj          |    4 +
 WebCore/page/DOMWindow.idl                         |  154 ++++++++++++++++++-
 WebCore/svg/SVGAnimateMotionElement.idl            |   31 ++++
 WebCore/svg/SVGAnimatedAngle.idl                   |    5 +-
 WebCore/svg/SVGAnimatedBoolean.idl                 |    5 +-
 WebCore/svg/SVGAnimatedEnumeration.idl             |    5 +-
 WebCore/svg/SVGAnimatedInteger.idl                 |    5 +-
 WebCore/svg/SVGAnimatedLength.idl                  |    5 +-
 WebCore/svg/SVGAnimatedLengthList.idl              |    5 +-
 WebCore/svg/SVGAnimatedNumber.idl                  |    5 +-
 WebCore/svg/SVGAnimatedNumberList.idl              |    5 +-
 WebCore/svg/SVGAnimatedPreserveAspectRatio.idl     |    5 +-
 WebCore/svg/SVGAnimatedRect.idl                    |    5 +-
 WebCore/svg/SVGAnimatedString.idl                  |    5 +-
 WebCore/svg/SVGAnimatedTransformList.idl           |    5 +-
 WebCore/svg/SVGElementInstance.idl                 |    1 +
 WebCore/svg/SVGElementInstanceList.idl             |    5 +-
 WebCore/svg/SVGLengthList.idl                      |    5 +-
 WebCore/svg/SVGMatrix.idl                          |    6 +-
 WebCore/svg/SVGNumber.idl                          |    6 +-
 WebCore/svg/SVGNumberList.idl                      |    5 +-
 WebCore/svg/SVGPathSegList.idl                     |    5 +-
 WebCore/svg/SVGPoint.idl                           |    6 +-
 WebCore/svg/SVGPointList.idl                       |    5 +-
 WebCore/svg/SVGRect.idl                            |    6 +-
 WebCore/svg/SVGStringList.idl                      |    5 +-
 WebCore/svg/SVGTransformList.idl                   |    5 +-
 WebCore/svg/SVGZoomAndPan.idl                      |    6 +-
 33 files changed, 514 insertions(+), 29 deletions(-)
Comment 9 Sam Weinig 2008-10-23 11:37:31 PDT
Comment on attachment 24572 [details]
Expose many missing constructors on window

> +        * svg/custom/global-constructors-expected.txt: Added.
> +        * svg/custom/global-constructors.html: Added.
You need to add the .js part of the test.


I think you should put this in its own patch as it is unrelated and needs its own test.
> +++ b/WebCore/svg/SVGAnimateMotionElement.idl
> @@ -0,0 +1,31 @@
> +/*
> + * Copyright (C) 2006 Apple Computer, Inc.
This should have your copyright.


> +module svg {
> +
> +    interface [Conditional=SVG_ANIMATION] SVGAnimateMotionElement : SVGAnimationElement {
This needs GenerateConstructor.
 
> -    interface [Conditional=SVG, GenerateConstructor, ObjCProtocol] SVGZoomAndPan {
> +    interface [
> +        Conditional=SVG,
> +        GenerateConstructor,
> +        ObjCProtocol
> +    ] SVGZoomAndPan {
I don't think this should have a constructor, at least until we decide what to do with pure interfaces.

Many additional classes need the GenerateConstructor extended attribute.  To make your life easier, you may want to reverse it, and make it necessary to opt out or a constructor.

r-
Comment 10 Eric Seidel (no email) 2009-04-29 12:37:54 PDT
*** Bug 22940 has been marked as a duplicate of this bug. ***
Comment 11 Eric Seidel (no email) 2009-07-15 14:19:14 PDT
Looking at this briefly today.
Comment 12 Eric Seidel (no email) 2009-07-15 14:19:47 PDT
Created attachment 32811 [details]
list of idls w/o GenerateConstructor
Comment 13 Nikolas Zimmermann 2009-12-24 16:31:03 PST
Created attachment 45478 [details]
Updated patch

This patch exposes all SVG 1.1 constructors, for all elements that are implemented.
SVG MI classes are not exposed, this is not possible at the moment (see bug 15494)

This patch is quite large, though it's just new layout test results.
Comment 14 WebKit Review Bot 2009-12-24 16:36:04 PST
style-queue ran check-webkit-style on attachment 45478 [details] without any errors.
Comment 15 WebKit Review Bot 2009-12-24 16:36:49 PST
Attachment 45478 [details] did not build on chromium:
Build output: http://webkit-commit-queue.appspot.com/results/144083
Comment 16 Nikolas Zimmermann 2009-12-24 17:01:15 PST
Created attachment 45479 [details]
Updated patch v2

Trying to fix chromium build with the help of the EWS.
Comment 17 WebKit Review Bot 2009-12-24 17:01:41 PST
style-queue ran check-webkit-style on attachment 45479 [details] without any errors.
Comment 18 WebKit Review Bot 2009-12-24 17:06:13 PST
Attachment 45479 [details] did not build on chromium:
Build output: http://webkit-commit-queue.appspot.com/results/144098
Comment 19 Nikolas Zimmermann 2009-12-24 17:37:14 PST
Created attachment 45481 [details]
Updated patch v3

I love EWS! Next attempt to fix v8 bindings aka. chromium build.
Comment 20 WebKit Review Bot 2009-12-24 17:37:42 PST
style-queue ran check-webkit-style on attachment 45481 [details] without any errors.
Comment 21 Adam Barth 2009-12-24 23:17:30 PST
> I love EWS! Next attempt to fix v8 bindings aka. chromium build.

Glad you're finding it useful.  We don't have a great way to report success, but your last patch did pass:

http://webkit-commit-queue.appspot.com/patch/45481
Comment 22 Dirk Schulze 2009-12-25 12:39:15 PST
Comment on attachment 45481 [details]
Updated patch v3

LGTM
Comment 23 Nikolas Zimmermann 2009-12-25 13:04:10 PST
Thanks, landed in r52559.