From e4a76812a675aa0a2f762fa09ab1278377848695 Mon Sep 17 00:00:00 2001
From: Sam <sam.saffron@gmail.com>
Date: Wed, 1 May 2013 16:37:27 +1000
Subject: [PATCH] this is a slightly round about way of making our self
 oneboxes sane shrunk avatar to 60px, added global whitelisting

---
 app/helpers/application_helper.rb             |  2 +-
 app/models/user.rb                            |  3 ++-
 lib/oneboxer.rb                               | 13 ++++++-----
 lib/oneboxer/base.rb                          | 10 ++++++++-
 lib/oneboxer/discourse_remote_onebox.rb       | 22 -------------------
 lib/oneboxer/handlebars_onebox.rb             | 12 ++++++++--
 lib/oneboxer/templates/simple_onebox.hbrs     |  2 +-
 lib/oneboxer/whitelist.rb                     |  1 +
 .../oneboxer/handlebars_onebox_spec.rb        | 17 ++++++++++++++
 spec/components/oneboxer/whitelist_spec.rb    | 18 +++++++++++++++
 10 files changed, 67 insertions(+), 33 deletions(-)
 delete mode 100644 lib/oneboxer/discourse_remote_onebox.rb
 create mode 100644 spec/components/oneboxer/handlebars_onebox_spec.rb
 create mode 100644 spec/components/oneboxer/whitelist_spec.rb

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}}
   <div class='onebox-result-body'>
     {{#image}}<img src="{{image}}" class="thumbnail">{{/image}}
-    <h3><a href="{{original_url}}" target="_blank">{{title}}</a></h3>
+    <h3><a href="{{original_url}}" target="_blank"{{#image_height}} height="{{image_height}}"{{/image_height}}{{#image_width}} width="{{image_width}}"{{/image_width}}>{{title}}</a></h3>
     {{#by_info}}<h4>{{by_info}}</h4>{{/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