Issue Details (XML | Word | Printable)

Key: MDL-17542
Type: Bug Bug
Status: Closed Closed
Resolution: Fixed
Priority: Major Major
Assignee: Francois Marier
Reporter: Francois Marier
Votes: 0
Watchers: 2
Operations

Add/Edit UI Mockup to this issue
If you were logged in you would be able to see more operations.
Moodle

html2text appears to be incompatible with the GPL

Created: 07/Dec/08 04:59 AM   Updated: 16/Jun/09 10:01 PM
Return to search
Component/s: Lib
Affects Version/s: 1.6.8, 1.7.6, 1.8.7, 1.9.3
Fix Version/s: 1.8.9, 1.9.5

File Attachments: 1. File html2text.new.php (20 kB)
2. File html2text.php (15 kB)
3. Text File html2text_replacement18.patch (28 kB)
4. Text File html2text_replacement19.patch (28 kB)
5. Text File html2text_replacement20.patch (28 kB)
Issue Links:
Dependency
 
Relates
 

URL: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=507947
Participants: Anthony Borrow, Dongsheng Cai, Eloy Lafuente (stronk7), Francois Marier, Helen Foster, Martin Dougiamas and Petr Skoda
Security Level: None
QA Assignee: Helen Foster
Difficulty: Moderate
Resolved date: 22/May/09
Affected Branches: MOODLE_16_STABLE, MOODLE_17_STABLE, MOODLE_18_STABLE, MOODLE_19_STABLE
Fixed Branches: MOODLE_18_STABLE, MOODLE_19_STABLE


 Description  « Hide
This library was reported in http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=507947

The license for html2text is not included in the code, but rather on the website of the author:

  http://www.howtocreate.co.uk/jslibs/termsOfUse.html

The most problematic bit of html2text's license is the requirement to get written permission to distribute:

"However, if you want to include the scripts [...] in bundled or distributed scripts/software (such as Opera or Dashboard widgets, or the Formativ Academic Timetable), you should contact me first and obtain my express written permission."

Then, there is a bit of discrimination for commercial sites through the addition of an "advertising clause":

"Commercial/profit-making public websites:

You should put a note saying that the script was provided for free by http://www.howtocreate.co.uk.

You put a plain text (ie. not script) link to my site on your policy page or other appropriate page, to help me with my search engine rankings."

The Debian developer who filed the bug suggested that we take a look at replacing this library with the one I have attached here. It's got the same name but is under the GPLv2+.

Based on a quick 1.9 grep, that file is only used in lib/weblib.php:html_to_text(), which in turn is only used in format_text_email() of the same library file.

 All   Comments   Change History   Version Control      Sort Order: Ascending order - Click to sort in descending order
[ Permalink | « Hide ]
Petr Skoda added a comment - 07/Dec/08 05:47 AM
thanks, going to review it later this week

Petr Skoda made changes - 07/Dec/08 05:47 AM
Field Original Value New Value
Fix Version/s 1.9.4 [ 10300 ]
Fix Version/s 2.0 [ 10122 ]
Eloy Lafuente (stronk7) made changes - 10/Dec/08 03:48 AM
Assignee moodle.com [ moodle.com ] Petr ?koda [ skodak ]
[ Permalink | « Hide ]
Eloy Lafuente (stronk7) added a comment - 10/Dec/08 03:48 AM
Assigning to Petr

[ Permalink | « Hide ]
Francois Marier added a comment - 16/Dec/08 02:05 PM
Here's a patch which replaces the html2text library with the attached GPL one.

The output is slightly different and we could help with more testing with different HTML inputs.

However, I will need to commit this to the Debian package today as it is considered a release-critical issue for Debian.


Francois Marier made changes - 16/Dec/08 02:05 PM
Attachment html2text_replacement.patch [ 15888 ]
[ Permalink | « Hide ]
Martin Dougiamas added a comment - 16/Dec/08 02:34 PM
I'm cool with the patch going into Lenny.

It can also go into HEAD for further testing (but not 1.9.x yet).


[ Permalink | « Hide ]
Martin Dougiamas added a comment - 16/Dec/08 02:39 PM
Passing to Dongsheng to run some performance comparisons (that's the main thing I'm worried about).

Martin Dougiamas made changes - 16/Dec/08 02:39 PM
Assignee Petr ?koda [ skodak ] Dongsheng Cai [ dongsheng ]
[ Permalink | « Hide ]
Dongsheng Cai added a comment - 16/Dec/08 03:22 PM
The new one is faster to process <p></p>
New: 0.0113739967346
Old: 0.183360099792
(500 p tags)

but slower to process <img and <table>
New: 0.755731105804
Old: 0.208492994308
(500 tables, 500 imgs and 500 a tags)


[ Permalink | « Hide ]
Francois Marier added a comment - 16/Dec/08 03:38 PM
> 500 a tags

There is a setting to disable the processing of links (create a list of URLs at the end I think). Perhaps we should disable that to make it faster?


[ Permalink | « Hide ]
Dongsheng Cai added a comment - 16/Dec/08 05:08 PM
Hi, Francois
Another test on links only:
New: 0.126650094986
Old: 0.449314832687

Actually, the new lib faster in everything except <img, the funnest thing is the new lib directly ignore img tags, should we make it support process img tags?


[ Permalink | « Hide ]
Dongsheng Cai added a comment - 16/Dec/08 05:20 PM
I modified this file, now, the new lib wins on all tests

Dongsheng Cai made changes - 16/Dec/08 05:20 PM
Attachment html2text.php [ 15889 ]
[ Permalink | « Hide ]
Francois Marier added a comment - 17/Dec/08 10:06 AM
Hi Dongsheng,

Here's a new version of the file. There are a few code changes, but the most important one is a security fix for possible code execution (see bottom of http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=508909 for the details).

Can you please run your script against this one as well?

Cheers,
Francois


Francois Marier made changes - 17/Dec/08 10:06 AM
Attachment html2text.new.php [ 15897 ]
[ Permalink | « Hide ]
Dongsheng Cai added a comment - 17/Dec/08 10:47 AM
both OOP html2text versions are fater than the proceduring html2text,
new version is a bit slower the the old version, I guess it use more regex in code.

Old OOP: 0.0414950847626

New OOP: 0.055447101593

proceduring: 0.39603304863

the testing text includes 500 links, images, tables and p tags each.


Francois Marier made changes - 17/Dec/08 11:34 AM
Attachment html2text.inc [ 15825 ]
Francois Marier made changes - 17/Dec/08 11:34 AM
Attachment html2text_replacement.patch [ 15888 ]
[ Permalink | « Hide ]
Francois Marier added a comment - 17/Dec/08 11:35 AM
New patch against 1.9 with the fixed version of html2text.php

Francois Marier made changes - 17/Dec/08 11:35 AM
Attachment html2text_replacement19.patch [ 15898 ]
[ Permalink | « Hide ]
Francois Marier added a comment - 17/Dec/08 11:36 AM
New patch against 1.8 with the new version of html2text.php

Francois Marier made changes - 17/Dec/08 11:36 AM
Attachment html2text_replacement18.patch [ 15899 ]
[ Permalink | « Hide ]
Francois Marier added a comment - 17/Dec/08 11:38 AM
New patch against HEAD with the fixed version of html2text.php

Francois Marier made changes - 17/Dec/08 11:38 AM
Attachment html2text_replacement20.patch [ 15900 ]
fmarier committed 2 files to 'Moodle CVS' - 17/Dec/08 11:55 AM
MDL-17542 weblib/html2text: replace html2text.php with a GPL alternative (which is also faster)
MODIFY lib/weblib.php   Rev. 1.1187    (+6 -6 lines)
MODIFY lib/html2text.php   Rev. 1.11    (+573 -145 lines)
Francois Marier made changes - 17/Dec/08 11:56 AM
Fix Version/s 1.9.4 [ 10300 ]
Assignee Dongsheng Cai [ dongsheng ] Francois Marier [ francois ]
Status Open [ 1 ] Resolved [ 5 ]
Resolution Fixed [ 1 ]
Anthony Borrow made changes - 25/Feb/09 03:12 PM
Link This issue will help resolve MDL-18368 [ MDL-18368 ]
[ Permalink | « Hide ]
Anthony Borrow added a comment - 25/Feb/09 03:13 PM
Francois - Since Martin's concern was with performance and the new function was shown to be faster and because of the licensing issues, should we not also apply the changes to 1.9 and 1.8? I came across this file when looking at another issue (MDL-18368). Applying the 1.9 patch cleared up the issue I was experiencing in MDL-18368 so it would resolve licensing, be faster, and less prone to PHP notices. I see no reason not to apply it to 18STABLE and 19STABLE. Peace - Anthony

[ Permalink | « Hide ]
Anthony Borrow added a comment - 25/Feb/09 03:15 PM
I am re-opening in the hopes that we can apply this to 1.8 and 1.9 and thus resolve MDL-18368. Thanks for your consideration. Peace - Anthony

Anthony Borrow made changes - 25/Feb/09 03:15 PM
Resolution Fixed [ 1 ]
Status Resolved [ 5 ] Reopened [ 4 ]
[ Permalink | « Hide ]
Anthony Borrow added a comment - 25/Feb/09 03:18 PM
One last thought/question, if the licensing is an issue for Debian will they not want this fixed in the 1.8 Moodle package?

[ Permalink | « Hide ]
Francois Marier added a comment - 25/Feb/09 03:51 PM
I don't mind applying it to 1.8 and 1.9. There are a few differences in the conversions from HTML to text but as far as I remember, it was a matter of the output being "different" (as opposed to being "wrong").

The Debian packages all use the new code already.

Francois


fmarier committed 2 files to 'Moodle CVS' on branch 'MOODLE_19_STABLE' - 03/Apr/09 07:00 AM
html2text: MDL-17542 switch to a GPL-compatible html2text library
MODIFY lib/html2text.php   Rev. 1.8.26.3    (+573 -145 lines)
MODIFY lib/weblib.php   Rev. 1.970.2.135    (+6 -6 lines)
fmarier committed 2 files to 'Moodle CVS' on branch 'MOODLE_18_STABLE' - 03/Apr/09 07:14 AM
html2text: MDL-17542 switch to a GPL-compatible html2text library
MODIFY lib/weblib.php   Rev. 1.812.2.110    (+9 -3 lines)
MODIFY lib/html2text.php   Rev. 1.8.20.1    (+573 -147 lines)
[ Permalink | « Hide ]
Francois Marier added a comment - 03/Apr/09 07:17 AM
Committed the fix to 1.8 and 1.9 stable.

Francois Marier made changes - 03/Apr/09 07:17 AM
Fix Version/s 1.8.9 [ 10322 ]
Resolution Fixed [ 1 ]
Fix Version/s 1.9.5 [ 10320 ]
Status Reopened [ 4 ] Resolved [ 5 ]
Fix Version/s 2.0 [ 10122 ]
Petr Skoda made changes - 21/May/09 06:43 PM
Link This issue has been marked as being related by MDL-19266 [ MDL-19266 ]
[ Permalink | « Hide ]
Petr Skoda added a comment - 21/May/09 06:44 PM
looks like we have a serious regression, please review the linked bug report, thanks

Petr Skoda made changes - 21/May/09 06:44 PM
Status Resolved [ 5 ] Reopened [ 4 ]
Resolution Fixed [ 1 ]
fmarier committed 3 files to 'Moodle CVS' on branch 'MOODLE_19_STABLE' - 22/May/09 10:07 AM
MDL-17542 lib/html2text: update to latest upstream version (r2374)

from http://trac.roundcube.net/log/trunk/roundcubemail/program/lib/html2text.php
MODIFY lib/html2text.php   Rev. 1.8.26.4    (+46 -47 lines)
MODIFY lib/weblib.php   Rev. 1.970.2.138    (+2 -5 lines)
MODIFY lib/html2text_readme.txt   Rev. 1.1.2.1    (+5 -0 lines)
fmarier committed 3 files to 'Moodle CVS' - 22/May/09 10:10 AM
MDL-17542 lib/html2text: update to latest upstream version (r2374)
from http://trac.roundcube.net/log/trunk/roundcubemail/program/lib/html2text.php
MODIFY lib/html2text_readme.txt   Rev. 1.2    (+5 -0 lines)
MODIFY lib/weblib.php   Rev. 1.1254    (+2 -5 lines)
MODIFY lib/html2text.php   Rev. 1.12    (+46 -47 lines)
[ Permalink | « Hide ]
Francois Marier added a comment - 22/May/09 10:11 AM
I have committed an updated version of this file to CVS, along with a readme describing its origin.

Francois Marier made changes - 22/May/09 10:11 AM
Resolution Fixed [ 1 ]
Status Reopened [ 4 ] Resolved [ 5 ]
fmarier committed 3 files to 'Moodle CVS' on branch 'MOODLE_18_STABLE' - 22/May/09 10:28 AM
MDL-17542 lib/html2text: update to latest upstream version (r2374)
from http://trac.roundcube.net/log/trunk/roundcubemail/program/lib/html2text.php
MODIFY lib/html2text_readme.txt   Rev. 1.2.2.2    (+5 -0 lines)
MODIFY lib/html2text.php   Rev. 1.8.20.2    (+46 -47 lines)
MODIFY lib/weblib.php   Rev. 1.812.2.111    (+2 -5 lines)
Petr Skoda made changes - 10/Jun/09 03:23 AM
Link This issue has been marked as being related by MDL-19447 [ MDL-19447 ]
[ Permalink | « Hide ]
Helen Foster added a comment - 16/Jun/09 10:01 PM
Thanks Francois

Helen Foster made changes - 16/Jun/09 10:01 PM
Status Resolved [ 5 ] Closed [ 6 ]
QA Assignee tsala

Powered by a free Atlassian JIRA open source license for Moodle. Try JIRA - bug tracking software for your team.
Atlassian JIRA the Professional Issue Tracker. (Enterprise Edition, Version: 3.13.5-#360) - Bug/feature request - Atlassian news - Contact Administrators