JS and CSS aggregation deletes license information
hass - July 2, 2007 - 11:52
| Project: | Drupal |
| Version: | 7.x-dev |
| Component: | base system |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | patch (code needs review) |
Description
The current JS compression in D5 and D6 delete the copyright and license information. We should keep this intact (and move to the top) for not getting a law problem.
Here is an example with http://cssdoc.net/ that is based on javadoc standard that can be used to read this automatically and looks nearly the same like the currently used PHP inline documentation in Drupal.
/**
* @copyright Copyright 2005-2007, Firstname Lastname
* @license CC-A 2.0 (http://creativecommons.org/licenses/by/2.0/)
*/
#1
#2
Marking as non-critical.
#3
#4
How would you identify a copyright comment? It is many times not in the @copyright form you use here. Look at the jquery comment for example.
#5
#6
The comments are currently not in this format - yes... but if the copyright information is required by law we are able to keep the information. i see no other way how it can be accomplished in a different way. We need to specify how comments this must look like and then we keep the notice intact (and move to the top). if someone adds normal inline comments in JS this comments will be removed as now. This is the expected behaviour for JS aggregation...
So i prefer to use the example as specification how this comments must looks like, while it is machine readable (what normal inline comments are not).
#7
How the copyright should look like is not specified by us, but the copyright holder, as in the case of the jquery copyright for example.
#8
Either we just include the first phpdoc comment of all files in the aggregated version, or we define some custom marker, like [copyright], [/copyright], which the aggregation function then collects into a new comment at the beginning of the aggregated file. The latter is "more secure", of course, but would require to add these copyright tags to all copyrighted files, e.g. to jquery.js.
#9
nobody makes us to keep his comment 100% intact, isn't it? We are made to tell "the copyright owner is..." and "this is licensed under..." nothing more. So as an example for jquery may this ends with:
Original notice:
/** jQuery 1.1.2 - New Wave Javascript
*
* Copyright (c) 2007 John Resig (jquery.com)
* Dual licensed under the MIT (MIT-LICENSE.txt)
* and GPL (GPL-LICENSE.txt) licenses.
*
*/
Aggregateable jquery notices:
/*** @copyright 2007 John Resig (jquery.com)
* @license Dual licensed under the MIT (MIT-LICENSE.txt) and GPL (GPL-LICENSE.txt) licenses.
*/
The comment should be keept as short as possible by law. name the owner, name the license that's it. don't insert the full license... we'd like to compress code and not keep it as big as it is :-).
#10
What we are required to keep and what we are able to modify depends on the exact license text, not some gut feelings. As long as it is not strengthened by a license quote, it is not possible to modify the exact copyright text used by the originating authors.
#11
Sorry, i have no clue what you are thinking about... :-) I'm sure we cannot guaranty that this solution will solve all variants today and for future, but nearly "all" and i thought about GPL, LGPL, CC and other variants - will work in this way very well. Other licenses may have issues with Drupal in general or are not allowed together with Drupal and therefore are no real issue. Are you thinking about a 0.01% variant?
I have often seen compressed JS files and all what copyright owners demanded - was one line like Copyright 2005-2007, Firstname Lastname on top. No one have written, we need to include a complete license text!? They understand what the intention is with compressing JS and CSS and nevertheless feel happy that we use their scripts... I think it is a good idea and to keep an additional license string intact, but cannot remember any compressed script with such a line... if someone does not allow alteration/compression of the JS code we can disable compression and keep the original file intact... drupal_add_js() have such a option, isn't it?
How do you like to solve this problem if not in this way?
#12
OK, seems like I was not understandable. The issue comes down to this: does the GPL says that we should keep the original copyright notice as-is, and we should not modify it? Does it say that we can modify it if we want? Does it say nothing about this topic? (Drupal only allows GPL contributions).
#13
From Drupal's LICENSE.txt:
1. You may copy and distribute verbatim copies of the Program'ssource code as you receive it, in any medium, provided that you
conspicuously and appropriately publish on each copy an appropriate
copyright notice and disclaimer of warranty; keep intact all the
notices that refer to this License and to the absence of any warranty;
and give any other recipients of the Program a copy of this License
along with the Program.
www.gnu.org has a FAQ (http://www.gnu.org/copyleft/gpl.html) that -- though I didn't see this specific question -- addresses similar issues. In it, the following notices are recommended:
To do so, attach the following notices to the program. It is safest to attach them to the start of each source file to most effectively state the exclusion of warranty; and each file should have at least the “copyright” line and a pointer to where the full notice is found.
<one line to give the program's name and a brief idea of what it does.>
Copyright (C) <year> <name of author>
This program is free software: you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
the Free Software Foundation, either version 3 of the License, or
(at your option) any later version.
This program is distributed in the hope that it will be useful,
but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
GNU General Public License for more details.
You should have received a copy of the GNU General Public License
along with this program. If not, see <http://www.gnu.org/licenses/>.
This sentence, however, probably accurately describes the bare minimum of information necessary on each file:
It is safest to attach them to the start of each source file to most effectively state the exclusion of warranty; and each file should have at least the “copyright” line and a pointer to where the full notice is found.This "barest minimum" is more or less what seems to be found on jquery.js:
/** jQuery 1.1.3 - New Wave Javascript
*
* Copyright (c) 2007 John Resig (jquery.com)
* Dual licensed under the MIT (MIT-LICENSE.txt)
* and GPL (GPL-LICENSE.txt) licenses.
*
* $Date: 2007-07-01 08:54:38 -0400 (Sun, 01 Jul 2007) $
* $Rev: 2200 $
*/
All that said, of course, bear in mind that I am not a lawyer, nor do I play one on tv.
#14
#15
This is not only about GPL, though. A custom theme might have a custom copyright notice (as e.g. bluebeach on drupal.org) that one would not want to lose due to compression. So, I think that having some tags in the original css/js files that the aggregator parses out would be the only way to accomplish this. The aggregator should then include the content from between these tags in a comment at the top of the aggregated stylesheet/script file.
#16
How about this: we list the aggregated JS/CSS files in the aggregated file itself, thereby mentioning that for specifics about licenses, you should look in those files (which are simply the originals).
#17
I'm not a lawer, but it looks like the copyright should be the minimum. If we'd like to provide more info, we add a small copyright notice or a link to the license... but i think it is not required to add such bug text like written in #13. Everybody knows if you reference "GPL" where the GPL is located and we don't need to write all this license text... other licenses are nearly the same. CC for e.g. provide Webpages with their licenses. As Developer you only need to construct a link to "your" CC license and everyone knows "open this link and you can read the appropriate license". Something wrong with this?
#18
The point made by Wim is good, anybody that knows the GPL (and other licenses) that know if that is a good enough solution?
#19
@hass: isn't that what my sugestion does? "linking" to the location of the license?
#20
+1 for Wim's idea
#21
yup, sounds a reasonable way
#22
Here is a patch implementing what Wim suggested.
#23
Please have a look on Drupal Coding Standards.
In short:
" * ".$GLOBALS['base_url']."/".$file."\n";to' * '. $GLOBALS['base_url'] .'/'. $file ."\n";#24
Ok, here we go again. :)
#25
There are still wrong string concatenations and malformed indents:
<?php+ // Add the url to the aggregated file.
+ $file_header .= ' * '.$GLOBALS['base_url'].'/'.$file."\n";
{...}
- file_save_data($data, $csspath .'/'. $filename, FILE_EXISTS_REPLACE);
+ file_save_data($file_header.$data, $csspath .'/'. $filename, FILE_EXISTS_REPLACE);
{...}
- $contents .= _drupal_compress_js(file_get_contents($path). ';');
+ $contents .= _drupal_compress_js(file_get_contents($path). ';');
?>
btw: String concatenation after file_get_contents($path) seemed to be wrong before and may be fixed herein, too.
Comments should always end with a dot.
<?php+ // Close the comment that list the aggregated files
?>
#26
In the cases where there are a line break in the string, what is the Drupal way of doing it?
$file_header .= " * The below content is aggregated from the following files.\n";or
$file_header .= ' * The below content is aggregated from the following files.'."\n";#27
You can use double quotes here.
However, if you'd use single quotes, string concatenation should look like
<?php'...following files.' . "\n";
?>
#28
Sorry for my ignorance.
I have read about string concatenations in the Drupal Coding Standards 4 times now, and I think i got it. The first time i read it, I was rather tired. :)
The Coding Standards says to place a space after the dot if the following term is not a quote. I see "\n" as a quote, and therefor there is no space before it. If this applies to only single-quote quotes, there is a need for two more spaces in the patch.
The rest of the problems, I think I have managed to fix.
#29
Looks great - however, it seems you accidently inserted white-space after
<?php- if ($info['preprocess']) {
+ if ($info['preprocess']) {
?>
After this quick fix, change the status to patch needs review, please.
#30
#31
I'm sorry, it seems we overlooked these accidentally changes:
<?php- $data = '';
-
+ $data = '';
+
{...}
-function drupal_build_js_cache($files, $filename) {
- $contents = '';
-
+function drupal_build_js_cache($files, $filename) {
+ $contents = '';
+
?>
However, patch applies correctly. Works as described. Changed a CSS file in the queue and reference list was successfully updated.
One more review and this is RTBC.
#32
Fixed, along with some other small things.
#33
I tried this patch on a D5 with 2 hunks (js compression), what i expected, but it works :-). The size of file will not grow much, what's good. Should we backport this for D5.2, too?
#34
Here is a patch for Drupal 5.
#35
we distribute drupal with license information. i don't think we are obligated to put a license into every page we serve.
#36
@moshe weitzman: Don't forget Themes with CC licenses or other licenses.
Aside, why has someone added the GPL license information to the misc/jquery.js file for D6 if this is not required? Someone from outside cannot see the license of CC after JS or CSS is compressed and may grap the code without notice...
#37
needs a re-roll.
#38
We shouldn't waste too much time and build this on top of http://drupal.org/node/113607.
#39
Is this still a issue, or something that should not be fixed?
#40
Still an issue.
#41
Reroll against HEAD.