Bug 20430 – DOMWindow is missing lots of SVG* constructor objects

• Status: RESOLVED
• Resolution: FIXED
• Component: SVG

[Holger Freyther]

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.

[Holger Freyther]

Created attachment 22858 [details]
Bind SVGElementConstructor as SVGElement

Add a patch, test case and test result.

[Mark Rowe (bdash)]

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.

[Mark Rowe (bdash)]

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.

[Mark Rowe (bdash)]

Comment on attachment 22858 [details]
Bind SVGElementConstructor as SVGElement

r=me, if you add a ChangeLog entry, and update the layout test results.

[Darin Adler]

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.

[Eric Seidel (no email)]

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"?

[Eric Seidel (no email)]

*** Bug 21806 has been marked as a duplicate of this bug. ***

[Eric Seidel (no email)]

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(-)

[Sam Weinig]

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-

[Eric Seidel (no email)]

*** Bug 22940 has been marked as a duplicate of this bug. ***

[Eric Seidel (no email)]

Looking at this briefly today.

[Eric Seidel (no email)]

Created attachment 32811 [details]
list of idls w/o GenerateConstructor

[Nikolas Zimmermann]

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.

[WebKit Review Bot]

style-queue ran check-webkit-style on attachment 45478 [details] without any errors.

[WebKit Review Bot]

Attachment 45478 [details] did not build on chromium:
Build output: http://webkit-commit-queue.appspot.com/results/144083

[Nikolas Zimmermann]

Created attachment 45479 [details]
Updated patch v2

Trying to fix chromium build with the help of the EWS.

[WebKit Review Bot]

style-queue ran check-webkit-style on attachment 45479 [details] without any errors.

[WebKit Review Bot]

Attachment 45479 [details] did not build on chromium:
Build output: http://webkit-commit-queue.appspot.com/results/144098

[Nikolas Zimmermann]

Created attachment 45481 [details]
Updated patch v3

I love EWS! Next attempt to fix v8 bindings aka. chromium build.

[WebKit Review Bot]

style-queue ran check-webkit-style on attachment 45481 [details] without any errors.

[Adam Barth]

> 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

[Dirk Schulze]

Comment on attachment 45481 [details]
Updated patch v3

LGTM

[Nikolas Zimmermann]

Thanks, landed in r52559.