Bug 11167 – SVG idls may be missing some constructors

• Component: SVG
• Status: RESOLVED FIXED

[Eric Seidel (no email)]

With the previous GlobalObject system from KDOM/KSVG2 the following constructors were exposed from the window object:

SVGException
SVGLength
SVGAngle
SVGColor
SVGUnitTypes
SVGTransform
SVGPaint
SVGGradientElement
SVGPreserveAspectRatio
SVGZoomAndPan
SVGMarkerElement

I'm not sure that any of those are really supposed to be exposed (based on the DOM spec), but we should investigate and decide.

[Rob Buis]

Created attachment 12030 [details]
First attempt

Well, this goes a long way. It may need some talking with WildFox for finetuning and
weinig for objC side of things though.
Cheers,

Rob.

[Eric Seidel (no email)]

Comment on attachment 12030 [details]
First attempt

The tests would be easier to debug if they used an expect()-like function which printed PASS or FAIL per-test.  (It could even not print FAIL and you could track a bool for the final PASS printing.)

Iface is an awful postfix.  "SVGZoomAndPanInterface" would be better.

I'm not sure I understand why we need SVGZoomAndPanInterface.  i guess the bindings expect to be able to instantiate one in order to grab the constants off of it?  That seems silly.

SVGZoomAndPan (if really needed) should be NonInheritable (I believe that exists in WTF).

+    my $name = $dataNode->name;
+    if ($name eq "SVGZoomAndPan") {
+      @{$dataNode->attributes} = ();
+    }

Eeek.  What a hack.  I'm sure we can find a better way.

I think your editor wrapped SVGElement to another line:
+    interface [Conditional=SVG, GenerateConstructor] SVGFEColorMatrixElement
+                                                     : SVGElement

Overall this is a good change, but the SVGZoomAndPan stuff needs to be sorted out before we land it.  You should chat with WildFox or andersca, they may have ideas about better ways to do this.  Weinig may also have an opinion.

[Rob Buis]

Created attachment 13118 [details]
Improved patch

This version has a better testcase. Also I think SVGUnitTypes and SVGZoomAndPan are handled better now.
Cheers,

Rob.

[Eric Seidel (no email)]

Comment on attachment 13118 [details]
Improved patch

There are a bunch of things which confuse me about this patch.

+    if ($class =~ /^SVGFE+/ or $class =~ /^SVGComponentTransferFunctionElement$/) {

I think that we intended to be .* but .+ would make more sense, or \w+ or \w+Element or something.

Why is SVGException have it's own Shared<SVGException> type?

Why does SVGUnitTypes loose its obj-c DOM file?

I think all of the js-svg-constructors tests which currently fail should not be commented out, but rather should just "fail" in the expected tests.  That way people know when they change things.

I'm sure I'd have other comments, but I have to run.

[Rob Buis]

Created attachment 13120 [details]
Improved patch II

incorporated Sam's suggestions.
Cheers,

Rob.

[Darin Adler]

Comment on attachment 13120 [details]
Improved patch II

We should not be checking in any files created by the bindings generation script.

[Rob Buis]

Hi Darin,

(In reply to comment #6)
> (From update of attachment 13120 [details] [edit])
> We should not be checking in any files created by the bindings generation
> script.

Do you have another suggestion? The problem is when autogenerating these two classes (SVGUnitTypes and SVGZoomAndPan) much more js code is created than is strictly needed, hence me editing it.
Cheers,

Rob.

[Rob Buis]

Going to pick this up again since it is so close to being finished! Maybe needs talking with weinig and/or WildFox.

[Nikolas Zimmermann]

(In reply to comment #8)
> Going to pick this up again since it is so close to being finished! Maybe needs
> talking with weinig and/or WildFox.
> 

I'm incorporating your layout test in my upcoming SVG DOM patch.
Will fix a lot more than just the missing constructors.

Just to avoid duplicated work :-)

Greetings,
Niko

[Rob Buis]

Hi Niko,

(In reply to comment #9)
> (In reply to comment #8)
> > Going to pick this up again since it is so close to being finished! Maybe needs
> > talking with weinig and/or WildFox.
> > 
> 
> I'm incorporating your layout test in my upcoming SVG DOM patch.
> Will fix a lot more than just the missing constructors.
> 
> Just to avoid duplicated work :-)

Good that you mention it!
Watch it though, this one is tricky IIRC due to SVG spec oddness :) Contact me if you can for this one, maybe after your exam? My internet connection at home works now, so I am available all weekend.
Cheers,

Rob.

[Nikolas Zimmermann]

Landed in r24169.