Summary: | SVG idls may be missing some constructors | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Eric Seidel (no email) <eric> | ||||||||
Component: | SVG | Assignee: | Nikolas Zimmermann <zimmermann> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | ||||||||||
Priority: | P4 | ||||||||||
Version: | 420+ | ||||||||||
Hardware: | Mac | ||||||||||
OS: | OS X 10.4 | ||||||||||
Attachments: |
|
Description
Eric Seidel (no email)
2006-10-05 06:59:57 PDT
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.
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.
Created attachment 13118 [details]
Improved patch
This version has a better testcase. Also I think SVGUnitTypes and SVGZoomAndPan are handled better now.
Cheers,
Rob.
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.
Created attachment 13120 [details]
Improved patch II
incorporated Sam's suggestions.
Cheers,
Rob.
Comment on attachment 13120 [details]
Improved patch II
We should not be checking in any files created by the bindings generation script.
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. Going to pick this up again since it is so close to being finished! Maybe needs talking with weinig and/or WildFox. (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 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. Landed in r24169. |