[kdenlive] Review Request 126796: online resource -logs into users freesound accounts to get permission for HQ audio. Fixes archive.org video search new dependancy KF5WebKit. (libkf5webkit5-dev)

[Roger Morton]

> On Jan. 20, 2016, 10:28 a.m., Jean-Baptiste Mardelle wrote:
> > Thanks for this patch. Some comments / questions:
> > Some files are missing a license header
> > The token is saved in $HOME/config/kde.org/kdenlive.conf is that a standard location for saving web tokens ?

Licence headers. Yep. Can fix that.
$HOME/config/kde.org/kdenlive.conf  - I dunno what the "standard" for web tokens is. I was just making use of the built in QSettings object and that is where it saves the data. Is this OK ?


> On Jan. 20, 2016, 10:28 a.m., Jean-Baptiste Mardelle wrote:
> > src/utils/freesound.cpp, line 317
> > <https://git.reviewboard.kde.org/r/126796/diff/1/?file=434205#file434205line317>
> >
> >     I'd prefer if you keep the previous format, calling start with: 
> >     
> >     m_previewProcess->start(app, parameters);
> >     
> >     Seems more secure to me, especially if the url arg is comming from an unsecure source... I am not sure how Qt handles the situation if the url is like;
> >     
> >     "http://test.org;rm -Rf *"

Right. Security and Url injection.  I will see if I can get the other format working. I think now I remember resolving a confilict on this part. And I guess I resolved it the wrong way.


- Roger


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


On Jan. 18, 2016, 1:54 p.m., Roger Morton wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126796/
> -----------------------------------------------------------
> 
> (Updated Jan. 18, 2016, 1:54 p.m.)
> 
> 
> Review request for Kdenlive and Vincent Pinon.
> 
> 
> Repository: kdenlive
> 
> 
> Description
> -------
> 
> online resource -logs into users freesound accounts to get permission for HQ audio. Fixes archive.org video search new dependancy KF5WebKit. (libkf5webkit5-dev)
> 
> 
> Diffs
> -----
> 
>   src/CMakeLists.txt 87d29d2 
>   src/qt-oauth-lib/logindialog.h PRE-CREATION 
>   src/qt-oauth-lib/logindialog.cpp PRE-CREATION 
>   src/qt-oauth-lib/oauth2.h PRE-CREATION 
>   src/qt-oauth-lib/oauth2.cpp PRE-CREATION 
>   src/ui/freesound_ui.ui 55a453d 
>   src/utils/abstractservice.h 0a6a4a7 
>   src/utils/archiveorg.h 5cdc84f 
>   src/utils/archiveorg.cpp 1be4d6f 
>   src/utils/freesound.h f72d145 
>   src/utils/freesound.cpp 932c47f 
>   src/utils/openclipart.h 7a4df1e 
>   src/utils/openclipart.cpp adbba68 
>   src/utils/resourcewidget.h c171e3c 
>   src/utils/resourcewidget.cpp 6453f7f 
> 
> Diff: https://git.reviewboard.kde.org/r/126796/diff/
> 
> 
> Testing
> -------
> 
> Built. Tested file downloads from freesound. Auhenticates to freesound. keeps authenticated until you delete $HOME/.config/kde.org/kdenlive.conf 
> Archive.org video search is now working. Display animiated .gif as the preview file from Archive.org videos.
> No longer have to click search button twice to get it to do the search the first time it runs
> Option to not use personal freesound account and just use HQ preview file instead.
> Downloads low quality preview file in background and does not notify you of it finishing.
> 
> 
> Thanks,
> 
> Roger Morton
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdenlive/attachments/20160120/63b6d4db/attachment.html>