On 12/08/2010 08:46 AM, Vincent Massol wrote: > Hi Sergiu, > > I don't like several of the changes here since I believe they're increasing > our technical debt so I'd like we find a better way. Here's some > feedback/ideas: > > 1) com.xpn.util.Util and com.xpn.xwiki.web.Utils are deprecated and shouldn't > be used. On the contrary we should work towards removing stuff from them, not > adding to them :) I see you've added a new method which isn't too good IMO. > > 2) The EscapeTool you've added to the velocity module doesn't belong there > IMO since I don't see why it would be restricted to Velocity. It's not about > a language limitation that would be only needed for Velocity. I believe other > scripts and even other java part of XWiki might need to escape XML. In > addition you haven't commented why there's a need to write a new class when > an escape tool already exists and is provided by the Velocity Tools project > (in a few days/months we'll wonder why). > > For 2) what I think is best is to add the escape code to the xwiki-xml module > and have a ScriptService to make it available to all scripts. BTW there's > already a XMLUtils in there and which already does xml escaping (and which I > don't like and it could a good time to extract part of its stuff into an > Escaper/Encoder component). > > So XML and HTML escaping should go in xwiki-xml IMO and URL escaping should > go in the xwiki-URL module (should we need them). > > WDYT? >
1. I extended EscapeTool so that all the existing escaping doesn't need to be updated to use $services.xml.escape instead of $escaptool.xml, which happens in a LOT of places. It will also contain a method for escaping wiki content, but I didn't have time to finish it yet. I agree that maybe writing the code directly in the new EscapeTool isn't the best solution, it could be moved into xwiki-xml, and EscapeTool.xml would simply call the right method from xwiki-xml. 2. I changed Utils because I was lazy and made the smallest change possible that would fix the problem. > > On Dec 8, 2010, at 4:47 AM, sdumitriu (SVN) wrote: > >> Author: sdumitriu >> Date: 2010-12-08 04:47:08 +0100 (Wed, 08 Dec 2010) >> New Revision: 33301 >> >> Added: >> >> platform/core/trunk/xwiki-velocity/src/main/java/org/xwiki/velocity/tools/EscapeTool.java >> >> platform/core/trunk/xwiki-velocity/src/test/java/org/xwiki/velocity/tools/EscapeToolTest.java >> Modified: >> platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/XWiki.java >> platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/util/Util.java >> platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/web/Utils.java >> >> platform/core/trunk/xwiki-rendering/xwiki-rendering-api/src/main/java/org/xwiki/rendering/internal/renderer/printer/XHTMLWriter.java >> >> platform/core/trunk/xwiki-velocity/src/main/java/org/xwiki/velocity/internal/DefaultVelocityConfiguration.java >> Log: >> XWIKI-5514: "apostrophe" character badly displayed in Internet Explorer >> XWIKI-5763: Remove entity encoding from UTF-8 text in XHTML >> Fixed. -- Sergiu Dumitriu http://purl.org/net/sergiu/ _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
