[Marble-devel] Review Request 121528: Fixing Weather plugin UI issues

[Bernhard Beschow]

> On Dez. 15, 2014, 8:47 nachm., Bernhard Beschow wrote:
> > src/plugins/render/weather/WeatherItem.cpp, line 512
> > <https://git.reviewboard.kde.org/r/121528/diff/1/?file=333334#file333334line512>
> >
> >     Why not fix the HTML itself, i.e. inserting "<img src=... />"?
> 
> Illya Kovalevskyy wrote:
>     I would leave that like it's done now, cos in case if we want to change some HTML markup later. Approach I used in code allows to avoid such codebase changes.

The author of the HTML probably didn't think of this kind of "SQL injection attack". Given that the HTML can still be styled independently using CSS, I prefer to insert the whole img tag here, as decribed above.


- Bernhard


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/121528/#review72091
-----------------------------------------------------------


On Dez. 15, 2014, 8:01 nachm., Illya Kovalevskyy wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121528/
> -----------------------------------------------------------
> 
> (Updated Dez. 15, 2014, 8:01 nachm.)
> 
> 
> Review request for Marble, Bernhard Beschow, Dennis Nienhüser, Torsten Rahn, and Sanjiban Bairagya.
> 
> 
> Repository: marble
> 
> 
> Description
> -------
> 
> Related GCI-2014 task: http://www.google-melange.com/gci/task/view/google/gci2014/5896006566674432
> 
> At the moment, degree symbol in weather items is broken on some systems. Secondly, if condition info about paticular weather point is N/A, broken icon is shown. And finally, for forcasts, Marble uses locale strings, but these are in lower-case (Monday, for instance, is spelled with upper 'M', not lower 'm'), so the patch takes care of this too. 
> 
> Bonus: little cosmetic changes
> 
> 
> Diffs
> -----
> 
>   src/plugins/render/weather/WeatherData.cpp 0f15ef9 
>   src/plugins/render/weather/WeatherItem.cpp bb351c2 
>   src/plugins/render/weather/data/weatherscreen.css 3c93964 
> 
> Diff: https://git.reviewboard.kde.org/r/121528/diff/
> 
> 
> Testing
> -------
> 
> Visual testing
> 
> 
> File Attachments
> ----------------
> 
> Proof
>   https://git.reviewboard.kde.org/media/uploaded/files/2014/12/15/5053e039-2841-4bbe-bfce-025829f9c431__Screen_Shot_2014-12-15_at_9.59.08_AM.png
> 
> 
> Thanks,
> 
> Illya Kovalevskyy
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/marble-devel/attachments/20141215/7b5b83dc/attachment.html>