Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reddit spout fixes #1033

Merged
merged 6 commits into from
Apr 21, 2018
Merged

Reddit spout fixes #1033

merged 6 commits into from
Apr 21, 2018

Conversation

jtojnar
Copy link
Member

@jtojnar jtojnar commented Apr 19, 2018

Allow wider range of URLs in Reddit spout, including absolute URLs and searches.

@jtojnar jtojnar added the spouts label Apr 19, 2018
@jtojnar jtojnar added this to the 2.19 milestone Apr 19, 2018
Previously, provided URL was appended to a string 'https://www.reddit.com/'
textually, resulting in a broken URL when an absolute URL was used.

This commit uses GuzzleHttp\Url to construct the URL correctly.
@jtojnar jtojnar force-pushed the reddit-request-fixes branch 3 times, most recently from bf48360 to 26452e1 Compare April 20, 2018 07:44
@toonn
Copy link

toonn commented Apr 20, 2018

Fixed #1032 for me.

Previously, .json extension was just appended to a URL which failed
when a query string was used (for example for search).

This commit changes the URL construnction so that only the path
segment is modified.
Reddit escapes the HTML in selftext_html attribute so we need to
unescape it.

https://github.com/reddit-archive/reddit/wiki/JSON
The Reddit spout tried to detect if an item points to imgur.com and
change the link directly to an image in that case. Unfortunately,
it was flaky, for example, gifv files resulted in 403 error.

Instead we are using the previews feature, which, as a bonus, also
works on sites other than imgur.
@jtojnar jtojnar merged commit 6ade1c9 into master Apr 21, 2018
@jtojnar jtojnar deleted the reddit-request-fixes branch August 14, 2018 10:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants