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/