The svg/ code needs housekeeping: 1. Make all copyright headers look identical - That involves adding missing Copyright (C) statements before the dates. - Renaming my old email address, from wildfox to zimmermann. - Some headers use stars at the beginning of each line in the copyright statement, remove that. 2. Cleanup headers - Remove class indention present in lots of SVG headers. - Remove unnecessary includes from the headers. - Remove the // namespace... and // ENABLE... lines after the #endif and namespace end brace. 3. Cleanup implementation files - Remove unncessary includes from each file (removing all, testing compilation by adding back one after the other) - Move inclusion of the corresponding header directly below the config.h line - Remove the // ENABLE... lines after the #endif - Fix general style issues pointed out by check-webkit-style I'm going to start immediately, it sucks to have an inconsistent coding style.
Note that this patch actually prepares an outstanding task: remove createWithNew from svgtags.in, deploy static PassRefPtr create() functionality to all SVG*Element classes, and let all start with refcount 1 instead of zero.
- Some headers use stars at the beginning of each line in the copyright statement, remove that. I would like it the other way arround, like we do it on every other non-SVG file. Means, allways use stars at the beginning.
- Remove the // ENABLE... lines after the #endif The same, please let them in, makes it much easier, if you don't use some kind of hyper modern code viewer to see which #endif closes which #if. And it's also in used on every non-SVG file. IIRC you also get some patches denied because you erased them (or at least got a comment about that).
Maybe we should discuss it on webkit-dev and write the result of this discussion to the style rules to end this religious war about '// ENABLE ...' and stars at the beginning of headers and cpp's. I think even we both had a dispute about it multiple times :-)
(In reply to comment #2) > - Some headers use stars at the beginning of each line in the copyright statement, remove that. > > I would like it the other way arround, like we do it on every other non-SVG file. Means, allways use stars at the beginning. Hm, okay, whatever is consistent. (In reply to comment #3) > - Remove the // ENABLE... lines after the #endif > > The same, please let them in, makes it much easier, if you don't use some kind of hyper modern code viewer to see which #endif closes which #if. And it's also in used on every non-SVG file. IIRC you also get some patches denied because you erased them (or at least got a comment about that). I don't think it makes anything easier. But you're right, most use it.
Created attachment 63306 [details] Header update script Adding the script I used to unify all header templates in svg/, might be of general interesst.
Created attachment 63307 [details] Patch - Updating all license headers There's no risk that I forgot to add a copyright in the new header template, as it's all auto-generated.
Comment on attachment 63307 [details] Patch - Updating all license headers > There's no risk that I forgot to add a copyright in the new header template, as it's all auto-generated. Nevertheless, I checked them ;-) Thank you Niko. This was the most important patch I've ever reviewed :-D
Well, I think you can not convert BSD licence to LGPL one. The BSD license says: 1. Redistributions of source code must retain the above copyright notice, this list of conditions and the following disclaimer. I am not a lawyer, but this means the copyright notice, the first and second point of condition list and the ending disclaimer must be in source code, but the new LGPL licence does not hold those.
(In reply to comment #9) > Well, I think you can not convert BSD licence to LGPL one. The BSD license says: > > 1. Redistributions of source code must retain the above copyright notice, this list of conditions and the following disclaimer. > > I am not a lawyer, but this means the copyright notice, the first and second point of condition list and the ending disclaimer must be in source code, but the new LGPL licence does not hold those. I didn't change a BSD license to a LGPL one, did I?
Oh you're right, I need permission from the author. For the IDL files it's easy, as I checked them in in r16337, obviously with the wrong license, as I intended to use LGPL (See http://trac.webkit.org/changeset/16337/trunk/WebCore/ksvg2/svg/SVGCircleElement.idl) CC'ing Darin Adler. I need to hear some comments whether I have to revert the license changes. FYI I did NOT notice that I actually changed some BSD -> LGPL licenses, sorry about that. Is it okay to change the licenses? Do I need to ask permission from anyone listed in the headers, that are affected?
Darin, can you please have a look?
> I didn't change a BSD license to a LGPL one, did I? :D for example: WebCore/svg/ElementTimeControl.h WebCore/svg/SVGAElement.idl WebCore/svg/SVGAllInOne.cpp ... etc ... Look for the '* 1. ' strings. ;)
Most IDL files contain the wrong copyright anyways, I'm the original author of those files, Eric imported them from KDE CVS more than 5 years ago (w/o licenses), and later ddkilzer added the standard Apple BSD license to them, which was wrong. Using trac you can find out that I'm the original author, so I can change the license. The full list of files that I accidently converted, w/o asking for permission are: svg/ElementTimeControl.h svg/ElementTimeControl.idl svg/SVGAllInOne.cpp svg/SVGAltGlyphElement.idl svg/SVGElementInstance.idl svg/SVGElementInstanceList.idl svg/SVGFEConvolveMatrix.idl svg/SVGFEMorphologyElement.idl svg/SVGFontElement.idl svg/SVGFontFaceElement.idl svg/SVGFontFaceFormatElement.idl svg/SVGFontFaceNameElement.idl svg/SVGFontFaceSrcElement.idl svg/SVGFontFaceUriElement.idl svg/SVGGlyphElement.idl svg/SVGMissingGlyphElement.idl svg/SVGViewSpec.idl svg/animation/SMILTime.cpp svg/animation/SMILTime.h svg/animation/SMILTimeContainer.cpp svg/animation/SMILTimeContainer.h svg/animation/SVGSMILElement.cpp svg/animation/SVGSMILElement.h svg/graphics/SVGImage.cpp svg/graphics/SVGImage.h svg/graphics/filters/SVGFELighting.cpp svg/graphics/filters/SVGFELighting.h Shall I revert these? Or ask the authors wheter it's fine?
(In reply to comment #14) > The full list of files that I accidently converted, w/o asking for permission are: > svg/ElementTimeControl.h > svg/ElementTimeControl.idl > svg/SVGAllInOne.cpp > svg/SVGAltGlyphElement.idl > svg/SVGElementInstance.idl > svg/SVGElementInstanceList.idl > svg/SVGFEConvolveMatrix.idl > svg/SVGFEMorphologyElement.idl > svg/SVGFontElement.idl > svg/SVGFontFaceElement.idl > svg/SVGFontFaceFormatElement.idl > svg/SVGFontFaceNameElement.idl > svg/SVGFontFaceSrcElement.idl > svg/SVGFontFaceUriElement.idl > svg/SVGGlyphElement.idl > svg/SVGMissingGlyphElement.idl > svg/SVGViewSpec.idl > svg/animation/SMILTime.cpp > svg/animation/SMILTime.h > svg/animation/SMILTimeContainer.cpp > svg/animation/SMILTimeContainer.h > svg/animation/SVGSMILElement.cpp > svg/animation/SVGSMILElement.h > svg/graphics/SVGImage.cpp > svg/graphics/SVGImage.h > svg/graphics/filters/SVGFELighting.cpp > svg/graphics/filters/SVGFELighting.h > > Shall I revert these? Or ask the authors wheter it's fine? If it is realy a BSD license, there are no strong arguments about changing the license here, since WebKit is dual licensed.
Comment on attachment 63307 [details] Patch - Updating all license headers Sorry, that I did not take the licensing into account enough. It is a BSD license and like I wrote in the previous comment, there is no reason to change the license of these files.
Dirk, you're aware this already landed?
> If it is realy a BSD license, there are no strong arguments about changing the license here, since WebKit is dual licensed. Yes, WebKit is dual licensed, but it does not mean that you can switch any license (BSD or LGPL) to the another one. > Sorry, that I did not take the licensing into account enough. It is a BSD license and like I wrote in the previous comment, there is no reason to change the license of these files. I would say there is no chance to switch a BSD license to a LGPL one (at least in this way ;) ).
(In reply to comment #17) > Dirk, you're aware this already landed? Eek, the comment did not get through (mid air collision) Landed "Patch - Updating all license headers" in r64539. Rolling it out, to be safe...
Created attachment 63336 [details] Patch - Updating all license headers (v2) I've updated my script to account for the different licenses: Now I'm unifying all LGPL files, and all BSD files, seperated.
FYI, all new code from Google or from Apple is very intentionally BSD instead of LGPL. Does RIM ask you to be adding new files under the LGPL? I'd be slightly surprised by that.
Comment on attachment 63336 [details] Patch - Updating all license headers (v2) The ChangeLog looks wrong. Otherwise this looks OK. This seems like a useful script to run over all of WebKit and to check into Scripts as tidy-licence-headers or something.
(In reply to comment #22) > (From update of attachment 63336 [details]) > The ChangeLog looks wrong. Otherwise this looks OK. I do not think so. I still do not understand why the ending disclaimers are changed at SVGFELighting* files. The University of Szeged and Apple Inc. are not the same group - at least for now ;).
(In reply to comment #23) > (In reply to comment #22) > > (From update of attachment 63336 [details] [details]) > > The ChangeLog looks wrong. Otherwise this looks OK. > > I do not think so. I still do not understand why the ending disclaimers are changed at SVGFELighting* files. The University of Szeged and Apple Inc. are not the same group - at least for now ;). And you introduced a space in: * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. ;-)
(In reply to comment #23) > (In reply to comment #22) > > (From update of attachment 63336 [details] [details]) > > The ChangeLog looks wrong. Otherwise this looks OK. > > I do not think so. I still do not understand why the ending disclaimers are changed at SVGFELighting* files. The University of Szeged and Apple Inc. are not the same group - at least for now ;). Good catch. I just replaced BSD style licenses with a newer template. That's why it replaced your copyright, I'm going to restore it.
(In reply to comment #24) > (In reply to comment #23) > > (In reply to comment #22) > > > (From update of attachment 63336 [details] [details] [details]) > > > The ChangeLog looks wrong. Otherwise this looks OK. > > > > I do not think so. I still do not understand why the ending disclaimers are changed at SVGFELighting* files. The University of Szeged and Apple Inc. are not the same group - at least for now ;). > > And you introduced a space in: > * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. > ;-) Yes, all other BSD licenses (most .idl files) have this trailing space, so it just saves svn diff length, to add the space in the other two files..
Landed patch v2 in r64579 - Gabor can you please have a look? I hope I resolved all your issues as well.
Your script is very ardent. ;) Look at WebCore/svg/SVGFEOffsetElement.h. In this file the LGPL licence is changed to BSD one.
(In reply to comment #28) > Your script is very ardent. ;) > > Look at WebCore/svg/SVGFEOffsetElement.h. > In this file the LGPL licence is changed to BSD one. *sigh*, fixed in r64633. Sorry for that :-) I hope that was the last failure?
This was approved over 4 months ago. Did it get landed?
At least the license texts were cleaned up. I think we can close this bug since the headline is a bit vague.