Review Request: Add HTML entity resolving to KNotify's XML-based HTML stripper

[Olivier Goffart]

Le Friday 10 September 2010, Sjors Gielen a écrit :
> > On 2010-09-10 06:28:29, Olivier Goffart wrote:
> > > good.
> 
> What do you think about backporting it to 4.5? I think it should be OK, no
> features added, just bugs fixed, in quite an easy way. Hard for me to
> test, though, so it would be best applied at least a few proofing days
> before the next 4.5 release.

Yes, it should be backported.



> 
> 
> - Sjors
> 
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://svn.reviewboard.kde.org/r/5229/#review7355
> -----------------------------------------------------------
> 
> On 2010-09-09 22:46:01, Sjors Gielen wrote:
> > -----------------------------------------------------------
> > This is an automatically generated e-mail. To reply, visit:
> > http://svn.reviewboard.kde.org/r/5229/
> > -----------------------------------------------------------
> > 
> > (Updated 2010-09-09 22:46:01)
> > 
> > 
> > Review request for kdelibs and Olivier Goffart.
> > 
> > 
> > Summary
> > -------
> > 
> > This patch adds HTML entity resolving.
> > The HTML stripper that I previously patched in KNotify is actually an XML
> > stripper; this is not much of a problem except many HTML entities like
> >   are not valid in XML, making the parser fail; this has the effect
> > that a message from an application with one such entity in it, is
> > displayed as plain HTML, instead of the stripped message. This patch
> > adds a HtmlEntityResolver class and makes the XML stripper use it to
> > resolve additional entities. The   which would have caused an XML
> > error, is now turned into a regular space, fixing the problem. (Any
> > unknown HTML entities are currently replaced with an empty string.)
> > 
> > I placed HtmlEntityResolver inside NotifyByPopup because it is such a
> > small class, but if I am going to add *all* HTML entities (there's a lot
> > of them), I think it would be best to split the file off into its own
> > class (unless there's a better class to use for this).
> > 
> > I think this patch should be backported to KDE 4.5 because it fixes an
> > important bug.
> > 
> > 
> > Diffs
> > -----
> > 
> >   /trunk/KDE/kdebase/runtime/knotify/notifybypopup.h 1173644
> >   /trunk/KDE/kdebase/runtime/knotify/notifybypopup.cpp 1173644
> > 
> > Diff: http://svn.reviewboard.kde.org/r/5229/diff
> > 
> > 
> > Testing
> > -------
> > 
> > Compiles and works on Mac OS X (tested with the Growl patch in 4.6
> > trunk). Only tested with  , not with other HTML entities, or a mix
> > of HTML and XML entities.
> > 
> > 
> > Thanks,
> > 
> > Sjors