> On 06 Sep 2016, at 16:59, Caleb James DeLisle <[email protected]> wrote: > > Hello all, > > Recently we discovered that in XWiki 8.2 the {{html}} macro cleaner now > removes <video> > tag whereas in 7.4 it did not and this has unfortunately caused problems for > the {{video}} > macro. > > 1. After some helpful investigation by a few XWiki developers, we have found > that in fact > the {{html}} macro is cleaning for XHTML and not for HTML5 which is what > XWiki uses and the > change has seemingly gone unnoticed since the upgrade to 8.0M1.
The HTML macro has always been cleaning to generate clean XHTML 1.0. This hasn’t changed. We do need to make it generate HTML5 for skins that output HTML5 but this hasn’t been done yet (see http://jira.xwiki.org/browse/XCOMMONS-901). What has happened (most likely) is that the HTML Cleaner library was updated from version 2.10 to 2.16 in 8.0M1 and somewhere in there the HTML Cleaner project must have fixed a bug where previously they were not stripping the video tag when they should have (since it’s not valid XHTML 1.0 probably). > 2. After some conversation with developers, I have observed that it is rare > for an application > developer to *intend* to have their HTML cleaned. In the XWiki repositories > we find that > amongst non-one-liner {{html}} macro invocations, 109 out of 292 (37%) of the > invocations > explicitly request clean=false [1]. Yes, developers should always use clean=false for 2 reasons: * performance * get an HTML validation error so that the error can be noticed and fixed in the HTML. Note: I hope that our validation tests are able to notice all the HTML errors and that as a consequence we won’t generate invalid X(HTML). FTR I wasn’t initially in agreement but got convinced by Marius, see https://github.com/xwiki-contrib/macro-video/commit/d986b8346a4c21d55472dd85e41a96d27f24680f#commitcomment-18260475 I even mentioned introducing a new {{rawhtml}} macro or something like this ({{cleanhtml}}, {{validhtml}}, …) if we want to avoid having to write clean=false. > 3. Nowhere is clean="true" specified and though one might think it obvious as > it is a default, > we find that wiki="false" (also default) is indeed specified 28 times [2]. That’s normal because you’re looking at development code and this code is supposed to generate valid HTML by default so there’s no need to clean it :) > 4. We see also that the HTML macro proves to be quite slow. During an > investigation of SOLR > performance Thomas found that "about 23% of the request is spend executing > html macros" [3]. > I suppose it should be obvious that without the cleaner, the {{html}} macro > should not cause > any significant performance degradation. > > > Given the status of this feature, I would like to propose that we make an > intentional change > of behavior and change the default to clean=false. This way we application > developers will not > need to type it all of the time and XWiki will become measurably faster. > > Here is my +1 for changing the default. We should not forget that the HTML macro is also meant to be used by users of the wiki (and not only by developers). BTW we even said in the past that we should try to avoid using the {{html}} macro at all in our code so that it can be disabled as a macro, see http://jira.xwiki.org/browse/XRENDERING-27 and even http://jira.xwiki.org/browse/XWIKI-7894 And most users don’t set the clean=false parameter. So I guess the question is whether we want to protect an XWiki instance against to generate invalid (X)HTML by default when the HTML macro is used by users, or whether we don’t care. TBH I don’t remember exactly why we introduced the HTML Cleaner but we probably had issues at some point. Anyone remembers? Thanks -Vincent > Thanks, > Caleb > > > > [1]: > $ grep -nr '{{html' | grep '.xml:' | grep 'clean=['\''"]*false' | grep -v > '{{/html}}' | wc -l > 109 > $ grep -nr '{{html' | grep '.xml:' | grep -v 'clean=['\''"]*false' | grep -v > '{{/html}}' | wc -l > 183 > > [2]: > $ grep -nr '{{html' | grep '.xml:' | grep 'wiki=['\''"]*false' | wc -l > 28 > > [3]: https://jira.xwiki.org/browse/XWIKI-12043 _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
