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

permit external URL resolution for images #32

Closed
wants to merge 1 commit into from
Closed

permit external URL resolution for images #32

wants to merge 1 commit into from

Conversation

danlobo
Copy link

@danlobo danlobo commented Mar 12, 2017

The actual code uses the url.openStream(), which doesn't give too much options to control how to obtain the image (from a local file, for example). This change allows the user to manipulate how to load URL images. The requirement is to create a subclass of URLResolver and its resolve method, returning the actual InputStream from the given url.

The URLResolver instance is found as a BrowserConfig's property.

@radkovo
Copy link
Owner

radkovo commented Mar 13, 2017

Many thanks for the PR. You are right that using url.openStream() is not extensible enough. But it seems to me that the URLResolver you propose duplicates the functionality of DocumentSource that is used for loading the remaining resources. It could be sufficient to adjust image loading to use DocumentSource as well. I have made a proposal in a separate imgload branch (see the last two commits). Does this solution meet your needs too?

@danlobo
Copy link
Author

danlobo commented Mar 13, 2017

Looking better at DocumentSource, you're right, it's the right class for this task. Your code meets my needs perfectly! Thanks for the corrections.

@radkovo
Copy link
Owner

radkovo commented Mar 14, 2017

Ok, I merged imgload to master and I am closing this PR.

@radkovo radkovo closed this Mar 14, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants