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

Create Imgur onebox #107

Merged
merged 13 commits into from Sep 28, 2013
Merged

Create Imgur onebox #107

merged 13 commits into from Sep 28, 2013

Conversation

jzeta
Copy link
Contributor

@jzeta jzeta commented Sep 2, 2013

No description provided.

@ghost ghost assigned vykster Sep 2, 2013
@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 15f5e86 on add-imgur-onebox into 5b8c6a5 on master.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 0fc9053 on add-imgur-onebox into 5b8c6a5 on master.

@krainboltgreene
Copy link

Needs to be able to handle albums.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 033e09c on add-imgur-onebox into 9be106f on master.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 9762601 on add-imgur-onebox into 9be106f on master.

@jzeta
Copy link
Contributor Author

jzeta commented Sep 21, 2013

@krainboltgreene I'll create another response for an album. I'll compare with the regular gallery response to determine how the onebox will be able to tell the difference.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 48fd862 on add-imgur-onebox into 9be106f on master.

@jzeta
Copy link
Contributor Author

jzeta commented Sep 23, 2013

@vykster reminded me that @markijbema's merged but ultimately deleted commit was for imgur onebox 91c6e05

include HTML

matches do
# /^https?\:\/\/imgur\.com\/.*$/
Copy link
Contributor

Choose a reason for hiding this comment

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

I understand this is a comment, but please now that ^ and $ don't do what you'd expect in Ruby. You mean (hopefully) \A and \Z.

Choose a reason for hiding this comment

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

We expect them to match the beginning of the line and the end of the line, respectively. That's what they do.

@markijbema
Copy link
Contributor

I have no particular interest in imgur though, mainly did that to play around with the discourse codebase. Did you check you support both albums and images? That's the only interesting part about it (though my fix didn't support that yet, nor did the old code).

I added three comments regarding security concerns. The latter two are about XSS, if you're already in a tag you don't need > or < to inject javascript, so regular sanitation doesn't work. normally.

@jzeta
Copy link
Contributor Author

jzeta commented Sep 25, 2013

Talked to @krainboltgreene, we should create 2 Imgur oneboxes, one for albums and another for galleries. Adding those to #129

@ghost ghost assigned jzeta Sep 28, 2013
jzeta added a commit that referenced this pull request Sep 28, 2013
@jzeta jzeta merged commit bbbc3d1 into master Sep 28, 2013
@jzeta jzeta deleted the add-imgur-onebox branch September 28, 2013 16:37
@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling c18dd3f on add-imgur-onebox into 88d5799 on master.

@jzeta
Copy link
Contributor Author

jzeta commented Sep 28, 2013

@vykster and I decided since Discourse only has support for Imgur images, we'll close this PR and add support for albums in v1.2.0. We'll look back at @markijbema's #valid_hash method then. Might consider using JSON parser since that's what Discourse is using, instead of HTML.

@jzeta jzeta removed their assignment Mar 20, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants