----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/100285/#review700 -----------------------------------------------------------
src/covermanager/CoverFetcher.cpp <http://git.reviewboard.kde.org/r/100285/#comment501> This is basically untested (unfortunately). src/network/NetworkAccessManagerProxy.cpp <http://git.reviewboard.kde.org/r/100285/#comment502> Redirects to the initial URL are handled here. However, infinite (redirection) loops are not correctly handled yet, but do we need that? src/scriptengine/AmarokNetworkScript.cpp <http://git.reviewboard.kde.org/r/100285/#comment503> This is needed as the slot which gets the result looks up some data in a QHash. The key of the QHash is the URL, but since the URL changed (due to the redirect) we have to update the URL (= the key) in the QHash. - Martin On 2011-01-02 20:25:45, Martin Blumenstingl wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/100285/ > ----------------------------------------------------------- > > (Updated 2011-01-02 20:25:45) > > > Review request for Amarok and Rick W. Chen. > > > Summary > ------- > > Currently NetworkAccessManagerProxy::getData() ignores redirects. > Since redirects are required for some URLs (see the linked bug) it would be > nice if Amarok's NetworkAccessManagerProxy would also support them. > > Please have a look at my comments below, I'm trying to explain some of the > issues there. > > Here's a quick overview of my changes: > -I implemented redirection support > -Infinite loops are currently not handled properly, if page A redirects to > itself my code will not do the redirect. > But my code does not handle "if page A redirects to page B which redirects > to page A"-loops. > -I tried to test the changes in CoverFetcher but I couldn't find anything > that uses redirects there. > Thus those changes are basically "untested". > > > This addresses bug 261839. > https://bugs.kde.org/show_bug.cgi?id=261839 > > > Diffs > ----- > > src/covermanager/CoverFetcher.h 9dc2f2e > src/covermanager/CoverFetcher.cpp b21dd87 > src/network/NetworkAccessManagerProxy.h 045ded3 > src/network/NetworkAccessManagerProxy.cpp 9a0e763 > src/scriptengine/AmarokNetworkScript.h 342cc2a > src/scriptengine/AmarokNetworkScript.cpp 1742f41 > > Diff: http://git.reviewboard.kde.org/r/100285/diff > > > Testing > ------- > > I tested Sven's "Free Music Charts" script: it works fine now. > Fetching lyrics through the "Ultimate Lyrics" script is also still working. > > > Thanks, > > Martin > >
_______________________________________________ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel
