think I have managed to show oEmbed images better now

classes/File.php

@@ -356,28 +356,47 @@ class File extends Managed_DataObject
         return $protocol.'://'.$server.$path.$filename;
     }
 
+    static $_enclosures = array();
+
     function getEnclosure(){
+        if (isset(self::$_enclosures[$this->getID()])) {
+            common_debug('Found cached enclosure for file id=='.$this->getID());
+            return self::$_enclosures[$this->getID()];
+        }
+
         $enclosure = (object) array();
         foreach (array('title', 'url', 'date', 'modified', 'size', 'mimetype') as $key) {
             $enclosure->$key = $this->$key;
         }
 
-        $needMoreMetadataMimetypes = array(null, 'application/xhtml+xml');
+        $needMoreMetadataMimetypes = array(null, 'application/xhtml+xml', 'text/html');
 
         if (!isset($this->filename) && in_array(common_bare_mime($enclosure->mimetype), $needMoreMetadataMimetypes)) {
             // This fetches enclosure metadata for non-local links with unset/HTML mimetypes,
             // which may be enriched through oEmbed or similar (implemented as plugins)
             Event::handle('FileEnclosureMetadata', array($this, &$enclosure));
         }
-        if (empty($enclosure->mimetype) || in_array(common_bare_mime($enclosure->mimetype), $needMoreMetadataMimetypes)) {
+        if (empty($enclosure->mimetype)) {
             // This means we either don't know what it is, so it can't
             // be shown as an enclosure, or it is an HTML link which
             // does not link to a resource with further metadata.
             throw new ServerException('Unknown enclosure mimetype, not enough metadata');
         }
+
+        self::$_enclosures[$this->getID()] = $enclosure;
         return $enclosure;
     }
 
+    public function hasThumbnail()
+    {
+        try {
+            $this->getThumbnail();
+        } catch (Exception $e) {
+            return false;
+        }
+        return true;
+    }
+
     /**
      * Get the attachment's thumbnail record, if any.
      * Make sure you supply proper 'int' typed variables (or null).

lib/attachmentlist.php

@@ -76,8 +76,8 @@ class AttachmentList extends Widget
     {
     	$attachments = $this->notice->attachments();
         foreach ($attachments as $key=>$att) {
-            // Only show attachments representable with a title
+            // Remove attachments which are not representable with neither a title nor thumbnail
-            if ($att->getTitle() === null) {
+            if ($att->getTitle() === null && !$att->hasThumbnail()) {
                 unset($attachments[$key]);
             }
         }

lib/attachmentlistitem.php

@@ -105,9 +105,24 @@ class AttachmentListItem extends Widget
     }
 
     function showRepresentation() {
+        $enclosure = $this->attachment->getEnclosure();
+
         if (Event::handle('StartShowAttachmentRepresentation', array($this->out, $this->attachment))) {
-            if (!empty($this->attachment->mimetype)) {
+            if (!empty($enclosure->mimetype)) {
-                $mediatype = common_get_mime_media($this->attachment->mimetype);
+                // First, prepare a thumbnail if it exists.
+                $thumb = null;
+                try {
+                    // Tell getThumbnail that we can show an animated image if it has one (4th arg, "force_still")
+                    $thumb = $this->attachment->getThumbnail(null, null, false, false);
+                } catch (UseFileAsThumbnailException $e) {
+                    $thumb = null;
+                } catch (UnsupportedMediaException $e) {
+                    // FIXME: Show a good representation of unsupported/unshowable images
+                    $thumb = null;
+                }
+
+                // Then get the kind of mediatype we're dealing with
+                $mediatype = common_get_mime_media($enclosure->mimetype);
 
                 // FIXME: Get proper mime recognition of Ogg files! If system has 'mediainfo', this should do it:
                 // $ mediainfo --inform='General;%InternetMediaType%'
@@ -117,27 +132,22 @@ class AttachmentListItem extends Widget
                 switch ($mediatype) {
                 // Anything we understand as an image, if we need special treatment, do it in StartShowAttachmentRepresentation
                 case 'image':
-                    try {
+                    if ($thumb instanceof File_thumbnail) {
-                        // Tell getThumbnail that we can show an animated image if it has one (4th arg, "force_still")
-                        $thumb = $this->attachment->getThumbnail(null, null, false, false);
                         $this->out->element('img', $thumb->getHtmlAttrs(['class'=>'u-photo', 'alt' => '']));
-                    } catch (UseFileAsThumbnailException $e) {
+                    } else {
-                        $this->out->element('img', array('class'=>'u-photo', 'src' => $e->file->getUrl(), 'alt' => $e->file->title));
+                        $this->out->element('img', array('class'=>'u-photo', 'src' => $this->attachment->getUrl(), 'alt' => $this->attachment->getTitle()));
-                    } catch (UnsupportedMediaException $e) {
-                        // FIXME: Show a good representation of unsupported/unshowable images
                     }
+                    unset($thumb);  // there's no need carrying this along after this
                     break;
 
                 // HTML5 media elements
                 case 'audio':
                 case 'video':
-                    try {
+                    if ($thumb instanceof File_thumbnail) {
-                        $thumb = $this->attachment->getThumbnail();
                         $poster = $thumb->getUrl();
-                        unset ($thumb);
+                        unset($thumb);  // there's no need carrying this along after this
-                    } catch (Exception $e) {
-                        $poster = null;
                     }
+
                     $this->out->elementStart($mediatype,
                                         array('class'=>"attachment_player u-{$mediatype}",
                                             'poster'=>$poster,
@@ -149,6 +159,7 @@ class AttachmentListItem extends Widget
                     break;
 
                 default:
+                    unset($thumb);  // there's no need carrying this along
                     switch ($this->attachment->mimetype) {
                     case 'text/html':
                         if (!empty($this->attachment->filename)

plugins/Oembed/OembedPlugin.php

@@ -187,7 +187,7 @@ class OembedPlugin extends Plugin
         return true;
     }
     
-    public function onStartShowAttachmentRepresentation(HTMLOutputter $out, File $file)
+    public function onShowUnsupportedAttachmentRepresentation(HTMLOutputter $out, File $file)
     {
         try {
             $oembed = File_oembed::getByFile($file);
@@ -195,6 +195,7 @@ class OembedPlugin extends Plugin
             return true;
         }
 
+        // the 'photo' type is shown through ordinary means, using StartShowAttachmentRepresentation!
         switch ($oembed->type) {
         case 'rich':
         case 'video':
@@ -207,15 +208,11 @@ class OembedPlugin extends Plugin
                     'elements'=>'*+object+embed');
                 $out->raw(htmLawed($oembed->html,$config));
             }
+            return false;
             break;
-
-        case 'photo':
-            $out->element('img', array('src' => $oembed->url, 'width' => $oembed->width, 'height' => $oembed->height, 'alt' => 'alt'));
-            break;
-
-        default:
-            Event::handle('ShowUnsupportedAttachmentRepresentation', array($out, $file));
         }
+
+        return true;
     }
 
     public function onCreateFileImageThumbnailSource(File $file, &$imgPath, $media=null)