Bug 20430

Created attachment 22858 [details] Bind SVGElementConstructor as SVGElement Add a patch, test case and test result.

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.

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 on attachment 22858 [details] Bind SVGElementConstructor as SVGElement r=me, if you add a ChangeLog entry, and update the layout test results.

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.

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

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

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

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

Looking at this briefly today.

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

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.

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

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

Created attachment 45479 [details] Updated patch v2 Trying to fix chromium build with the help of the EWS.

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

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

Created attachment 45481 [details] Updated patch v3 I love EWS! Next attempt to fix v8 bindings aka. chromium build.

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

> 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 on attachment 45481 [details] Updated patch v3 LGTM

Thanks, landed in r52559.