Skip to content
This repository has been archived by the owner on Jun 9, 2023. It is now read-only.

Always show fullsize imgur images #209

Merged
merged 4 commits into from
May 20, 2020

Conversation

AbsurdlySuspicious
Copy link
Collaborator

When submission contains direct imgur url sometimes Dawn were loading preview images. _d filename suffix and .webp extension both required to reproduce this. I've added webp as image format and also added stripping of _d just to be safe

Copy link
Owner

@Tunous Tunous left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you check whether there are tests in UrlParser, and if it would be possible to add new tests for your changes?
It’s not a requirement but would be nice to have.

// Strip any preview-related suffixes and queries
imageUrl = imageUrl.newBuilder()
.encodedQuery(null)
.encodedPath(imgurPreviewPat.matcher(imageUrlPath).replaceFirst("."))
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It probably won’t occur but what if that pattern was found earlier than before extension? Maybe it’s better to replace last occurrence instead.

Copy link
Collaborator Author

@AbsurdlySuspicious AbsurdlySuspicious May 19, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using java's matcher this can only be done by a more precise regex. Something like s/_d(\.\w+)$/$1/

@AbsurdlySuspicious
Copy link
Collaborator Author

yes, I'll add test cases for this

@AbsurdlySuspicious
Copy link
Collaborator Author

that should do it

@Tunous
Copy link
Owner

Tunous commented May 20, 2020

Great, thanks.

@Tunous Tunous merged commit d7a73d9 into Tunous:master May 20, 2020
@AbsurdlySuspicious AbsurdlySuspicious deleted the imgur_lowres/feature branch June 2, 2020 17:28
@AbsurdlySuspicious
Copy link
Collaborator Author

By the way, maybe it worth to add _d and ?maxwidth= for lowQualityUrl of ImgurLink to load smaller image when data saver is enabled in settings?

@Tunous
Copy link
Owner

Tunous commented Jun 13, 2020

Sounds like a good idea for improvement.

@Tunous Tunous added the bug Something isn't working label Aug 2, 2020
@Tunous Tunous added this to the 0.9.2 milestone Aug 2, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants