diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 1f67a704b26..413328f0220 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -43,7 +43,7 @@ module ApplicationHelper result = tag(:meta, property: 'og:site_name', content: SiteSetting.title) << "\n" result << tag(:meta, name: 'twitter:card', content: "summary") - [:image, :url, :title, :description].each do |property| + [:image, :url, :title, :description, 'image:width', 'image:height'].each do |property| if opts[property].present? escape = (property != :image) result << tag(:meta, {property: "og:#{property}", content: opts[property]}, nil, escape) << "\n" diff --git a/app/models/user.rb b/app/models/user.rb index 90813a20bc5..e8f12a8d25d 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -351,8 +351,9 @@ class User < ActiveRecord::Base end # Don't pass this up to the client - it's meant for server side use + # The only spot this is now used is for self oneboxes in open graph data def small_avatar_url - "https://www.gravatar.com/avatar/#{email_hash}.png?s=200&r=pg&d=identicon" + "https://www.gravatar.com/avatar/#{email_hash}.png?s=60&r=pg&d=identicon" end # return null for local avatars, a template for gravatar diff --git a/lib/oneboxer.rb b/lib/oneboxer.rb index f8810a193ee..046f1891af0 100644 --- a/lib/oneboxer.rb +++ b/lib/oneboxer.rb @@ -51,6 +51,9 @@ module Oneboxer whitelist_entry = Whitelist.entry_for_url(url) if whitelist_entry.present? + # TODO - only download HEAD section + # TODO - sane timeout + # TODO - FAIL if for any reason you are downloading more that 5000 bytes page_html = open(url).read if page_html.present? doc = Nokogiri::HTML(page_html) @@ -102,14 +105,14 @@ module Oneboxer onebox, preview = yield(url,element) if onebox parsed_onebox = Nokogiri::HTML::fragment(onebox) - next unless parsed_onebox.children.count > 0 + next unless parsed_onebox.children.count > 0 # special logic to strip empty p elements - if element.parent && - element.parent.node_name.downcase == "p" && - element.parent.children.count == 1 && + if element.parent && + element.parent.node_name.downcase == "p" && + element.parent.children.count == 1 && parsed_onebox.children.first.name.downcase == "div" - element = element.parent + element = element.parent end changed = true element.swap parsed_onebox.to_html diff --git a/lib/oneboxer/base.rb b/lib/oneboxer/base.rb index 19bbb01aaa4..f0fb9373092 100644 --- a/lib/oneboxer/base.rb +++ b/lib/oneboxer/base.rb @@ -4,7 +4,7 @@ module Oneboxer def parse_open_graph(doc) result = {} - %w(title type image url description).each do |prop| + %w(title type image url description image:width image:height).each do |prop| node = doc.at("/html/head/meta[@property='og:#{prop}']") result[prop] = (node['content'] || node['value']) if node end @@ -24,6 +24,14 @@ module Oneboxer result['description'] = node['content'] if node end + %w(image:width image:height).each do |prop| + # Some sane max width + if result[prop] && result[prop].to_i < 100 + result[prop.sub(":","_")] = result[prop] + end + result[prop] = nil + end + result end end diff --git a/lib/oneboxer/discourse_remote_onebox.rb b/lib/oneboxer/discourse_remote_onebox.rb deleted file mode 100644 index 34e2c90de5b..00000000000 --- a/lib/oneboxer/discourse_remote_onebox.rb +++ /dev/null @@ -1,22 +0,0 @@ -require_dependency 'oneboxer/oembed_onebox' -require_dependency 'freedom_patches/rails4' - -module Oneboxer - class DiscourseRemoteOnebox < HandlebarsOnebox - - matcher /^https?\:\/\/meta\.discourse\.org\/.*$/ - favicon 'discourse.png' - - def template - template_path('simple_onebox') - end - - def parse(data) - doc = Nokogiri::HTML(data) - open_graph = Oneboxer.parse_open_graph(doc) - open_graph['text'] = open_graph['description'] - open_graph - end - - end -end diff --git a/lib/oneboxer/handlebars_onebox.rb b/lib/oneboxer/handlebars_onebox.rb index f000a500aa9..3411bbc7da6 100644 --- a/lib/oneboxer/handlebars_onebox.rb +++ b/lib/oneboxer/handlebars_onebox.rb @@ -7,10 +7,14 @@ module Oneboxer MAX_TEXT = 500 - def template_path(template_name) + def self.template_path(template_name) "#{Rails.root}/lib/oneboxer/templates/#{template_name}.hbrs" end + def template_path(template_name) + HandlebarsOnebox.template_path(template_name) + end + def template template_name = self.class.name.underscore template_name.gsub!(/oneboxer\//, '') @@ -34,13 +38,17 @@ module Oneboxer args[:favicon] = ActionController::Base.helpers.image_path(self.class.favicon_file) if self.class.favicon_file.present? args[:host] = nice_host - Mustache.render(File.read(template), args) + HandlebarsOnebox.generate_onebox(template,args) rescue => ex # If there's an exception, just embed the link raise ex if Rails.env.development? default_url end + def self.generate_onebox(template, args) + Mustache.render(File.read(template), args) + end + end end diff --git a/lib/oneboxer/templates/simple_onebox.hbrs b/lib/oneboxer/templates/simple_onebox.hbrs index 13983175d6b..80c433a5100 100644 --- a/lib/oneboxer/templates/simple_onebox.hbrs +++ b/lib/oneboxer/templates/simple_onebox.hbrs @@ -10,7 +10,7 @@ {{/host}}
{{#image}}{{/image}} -

{{title}}

+

{{title}}

{{#by_info}}

{{by_info}}

{{/by_info}} {{#unsafe}} {{text}} diff --git a/lib/oneboxer/whitelist.rb b/lib/oneboxer/whitelist.rb index 6a18cd55d32..c911cfc3416 100644 --- a/lib/oneboxer/whitelist.rb +++ b/lib/oneboxer/whitelist.rb @@ -77,6 +77,7 @@ module Oneboxer Entry.new(/^https?:\/\/(?:www\.)?tumblr\.com\/.+/, false), Entry.new(/^https?:\/\/(?:www\.)?howtogeek\.com\/.+/, false), Entry.new(/\/\d{4}\/\d{2}\/\d{2}\//, false), # wordpress + Entry.new(/^https?:\/\/[^\/]+\/t\/[^\/]+\/\d+(\/\d+)?(\?.*)?$/) ] end diff --git a/spec/components/oneboxer/handlebars_onebox_spec.rb b/spec/components/oneboxer/handlebars_onebox_spec.rb new file mode 100644 index 00000000000..d8bbde3890f --- /dev/null +++ b/spec/components/oneboxer/handlebars_onebox_spec.rb @@ -0,0 +1,17 @@ +require 'spec_helper' +require 'oneboxer' +require 'oneboxer/handlebars_onebox' + +describe Oneboxer::HandlebarsOnebox do + H = Oneboxer::HandlebarsOnebox + + describe 'simple onebox' do + it "is able to render image size when specified" do + template = H.template_path('simple_onebox') + result = H.generate_onebox(template, 'image_width' => 100, 'image_height' => 100, image: 'http://my.com/image.png') + + result.should =~ /width=/ + result.should =~ /height=/ + end + end +end diff --git a/spec/components/oneboxer/whitelist_spec.rb b/spec/components/oneboxer/whitelist_spec.rb new file mode 100644 index 00000000000..bfc181cd0c7 --- /dev/null +++ b/spec/components/oneboxer/whitelist_spec.rb @@ -0,0 +1,18 @@ +require 'spec_helper' +require 'oneboxer' +require 'oneboxer/whitelist' + +describe Oneboxer::Whitelist do + it "matches an arbitrary Discourse post link" do + Oneboxer::Whitelist.entry_for_url('http://meta.discourse.org/t/scrolling-up-not-loading-in-firefox/3340/6?123').should_not be_nil + end + + it "matches an arbitrary Discourse topic link" do + Oneboxer::Whitelist.entry_for_url('http://meta.discourse.org/t/scrolling-up-not-loading-in-firefox/3340?123').should_not be_nil + end + + it "Does not match on slight variation" do + Oneboxer::Whitelist.entry_for_url('http://meta.discourse.org/t/scrolling-up-not-loading-in-firefox/3340a?123').should be_nil + end + +end