Review Request: Image Plugin

Lasath Fernando kde at lasath.org
Sat Jun 23 01:51:25 UTC 2012



> On June 21, 2012, 9:38 a.m., David Edmundson wrote:
> > filters/images/images-filter.cpp, line 59
> > <http://git.reviewboard.kde.org/r/105313/diff/1/?file=70001#file70001line59>
> >
> >     is that alt text really explaining anything?
> >     
> >     If you had a screen reader would it help?
> >     Also it's  user presented string, therefore needs i18n().
> >     
> >

Well, what should it say?


> On June 21, 2012, 9:38 a.m., David Edmundson wrote:
> > lib/message-processor.cpp, line 54
> > <http://git.reviewboard.kde.org/r/105313/diff/1/?file=70003#file70003line54>
> >
> >     what is this?
> >     
> >     you load all the plugins in any order, then you prepend the UrlFilter, then you loop through the list, find the escape filter and move that to the front....?
> >     
> >     You need to sort them when you load, maybe by some value in the .desktop file.
> >     
> >     Also is UrlFilter now in the filter twice?
> >     
> >

This is temporary. 
The UrlFilter had to run before ImagePlugin, otherwise there would be no URLs in the metadata. But this meant that the links it puts in were getting escaped by the EscapeFilter.

So, I did a little hack to make EsacpeFilter run first, then UrlFilter. I'll replace this when I figure out how to read properties from service files.


- Lasath


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/105313/#review14941
-----------------------------------------------------------


On June 21, 2012, 4:43 a.m., Lasath Fernando wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/105313/
> -----------------------------------------------------------
> 
> (Updated June 21, 2012, 4:43 a.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Description
> -------
> 
> This is a plugin to Embed Images directly into a chat. (I pretty much rewrote the old code from scratch because I realized I wasn't very happy with it).
> 
> I've also bumped the version number of all the plugins. 
> 
> This diff depends on diff of the previous review (which I still haven't merged yet). I'll merge them both once this gets reviewed. 
> 
> 
> Diffs
> -----
> 
>   filters/images/ktptextui_message_filter_images.desktop PRE-CREATION 
>   lib/message-processor.cpp a9b409e 
>   filters/images/images-filter.cpp PRE-CREATION 
>   filters/escape/ktptextui_message_filter_escape.desktop PRE-CREATION 
>   filters/formatting/format-filter.cpp PRE-CREATION 
>   filters/formatting/ktptextui_message_filter_formatting.desktop PRE-CREATION 
>   filters/images/CMakeLists.txt PRE-CREATION 
>   filters/images/images-filter.h PRE-CREATION 
>   filters/CMakeLists.txt PRE-CREATION 
>   filters/emoticons/ktptextui_message_filter_emoticons.desktop PRE-CREATION 
>   tests/message-processor-basic-tests.h PRE-CREATION 
>   tests/message-processor-basic-tests.cpp PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/105313/diff/
> 
> 
> Testing
> -------
> 
> Passed Unit tests, saw memes in chat. 
> 
> 
> Thanks,
> 
> Lasath Fernando
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-telepathy/attachments/20120623/9c57a408/attachment.html>


More information about the KDE-Telepathy mailing list