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

Add option to embed images from external domains #44

Merged
merged 1 commit into from
Feb 1, 2016
Merged

Add option to embed images from external domains #44

merged 1 commit into from
Feb 1, 2016

Conversation

bertpoort
Copy link
Contributor

Mantis id 0017997

@bertpoort
Copy link
Contributor Author

While browsing through the Scrutinizer output I noticed a typo on line 18: $destionationemail should be $destinationemail. The rest of the file uses the correct $destinationemail.

@michield
Copy link
Member

michield commented Feb 1, 2016

Great, as you put a flag around it "EMBEDEXTERNALIMAGES", I will merge and then we can take it from there. I will probably leave it off for the time being and we can see if we can get some feedback from people.

michield added a commit that referenced this pull request Feb 1, 2016
Add option to embed images from external domains
@michield michield merged commit c92e77f into phpList:master Feb 1, 2016
@michield
Copy link
Member

michield commented Feb 1, 2016

@bertpoort
Copy link
Contributor Author

Great. When it's ready for production use it probably could use some extra configuration parameters and possibly better default values for the following:

  • Max age for cached file (currently 1 day)
  • Max size for cached file (currently 1 MB)

Also note that in a worst case scenario this could pause sending of the first message for 30 seconds per external image since that's what the HTTP timeout values are set to. Errors are cached so only the first message would be affected.

@bertpoort bertpoort deleted the patch-embedexternalimages branch February 19, 2016 23:19
@bertpoort bertpoort restored the patch-embedexternalimages branch February 19, 2016 23:36
@bertpoort bertpoort deleted the patch-embedexternalimages branch February 19, 2016 23:36
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.

2 participants