Review Request: Make NetworkAccessManagerProxy::getData honor redirects

Martin Blumenstingl darklight.xdarklight at googlemail.com
Sun Jan 2 21:25:46 CET 2011


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

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

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/amarok-devel/attachments/20110102/18e1b006/attachment.htm 


More information about the Amarok-devel mailing list