Merge branch 'rich-media-fixes' into 'master'

Rich media parsing fixes

See merge request framasoft/mobilizon!924

js/src/components/Comment/CommentTree.vue

@@ -48,8 +48,9 @@
     >
       {{ $t("Loading comments…") }}
     </p>
-    <transition name="comment-empty-list" mode="out-in" v-else>
+    <transition-group name="comment-empty-list" mode="out-in" v-else>
       <transition-group
+        key="list"
         name="comment-list"
         v-if="comments.length"
         class="comment-list"
@@ -65,10 +66,10 @@
           @delete-comment="deleteComment"
         />
       </transition-group>
-      <div class="no-comments">
+      <div class="no-comments" key="no-comments">
         <span>{{ $t("No comments yet") }}</span>
       </div>
-    </transition>
+    </transition-group>
   </div>
 </template>
 

js/src/components/Resource/ResourceItem.vue

@@ -127,7 +127,6 @@ a {
       white-space: nowrap;
       display: inline-block;
       font-weight: 500;
-      margin-bottom: 5px;
       color: $primary;
       overflow: hidden;
       text-overflow: ellipsis;

js/src/views/Group/Group.vue

@@ -31,7 +31,11 @@
         {{ $t("You have been removed from this group's members.") }}
       </b-message>
       <b-message
-        v-if="isCurrentActorAGroupMember && isCurrentActorARecentMember"
+        v-if="
+          isCurrentActorAGroupMember &&
+          isCurrentActorARecentMember &&
+          isCurrentActorOnADifferentDomainThanGroup
+        "
         type="is-info"
       >
         {{
@@ -735,6 +739,10 @@ export default class Group extends mixins(GroupMixin) {
     );
   }
 
+  get isCurrentActorOnADifferentDomainThanGroup(): boolean {
+    return this.group.domain !== null;
+  }
+
   get members(): IMember[] {
     return this.group.members.elements.filter(
       (member) =>

js/tests/unit/specs/components/Comment/__snapshots__/CommentTree.spec.ts.snap

@@ -3,25 +3,25 @@
 exports[`CommentTree renders a comment tree 1`] = `
 <div>
   <!---->
-  <transition-stub name="comment-empty-list" mode="out-in">
+  <transition-group-stub name="comment-empty-list" mode="out-in">
     <transition-group-stub tag="ul" name="comment-list" class="comment-list">
       <comment-stub comment="[object Object]" event="[object Object]" class="root-comment"></comment-stub>
       <comment-stub comment="[object Object]" event="[object Object]" class="root-comment"></comment-stub>
     </transition-group-stub>
     <div class="no-comments"><span>No comments yet</span></div>
-  </transition-stub>
+  </transition-group-stub>
 </div>
 `;
 
 exports[`CommentTree renders a comment tree 2`] = `
 <div>
   <!---->
-  <transition-stub name="comment-empty-list" mode="out-in">
+  <transition-group-stub name="comment-empty-list" mode="out-in">
     <transition-group-stub tag="ul" name="comment-list" class="comment-list">
       <comment-stub comment="[object Object]" event="[object Object]" class="root-comment"></comment-stub>
       <comment-stub comment="[object Object]" event="[object Object]" class="root-comment"></comment-stub>
     </transition-group-stub>
     <div class="no-comments"><span>No comments yet</span></div>
-  </transition-stub>
+  </transition-group-stub>
 </div>
 `;

lib/service/rich_media/favicon.ex

@@ -56,19 +56,11 @@ defmodule Mobilizon.Service.RichMedia.Favicon do
 
   @spec format_url(String.t(), String.t()) :: String.t()
   defp format_url(url, path) do
-    image_uri = URI.parse(path)
-    uri = URI.parse(url)
-
-    cond do
-      is_nil(image_uri.host) -> "#{uri.scheme}://#{uri.host}#{correct_path(path)}"
-      is_nil(image_uri.scheme) -> "#{uri.scheme}:#{path}"
-      true -> path
+    url
+    |> URI.parse()
+    |> URI.merge(path)
+    |> to_string()
   end
-  end
-
-  # Sometimes paths have "/" in front, sometimes not
-  defp correct_path("/" <> _ = path), do: path
-  defp correct_path(path), do: "/#{path}"
 
   @spec find_favicon_link_tag(String.t()) :: {:ok, tuple()} | {:error, any()}
   defp find_favicon_link_tag(html) do

lib/service/rich_media/parser.ex

@@ -19,6 +19,7 @@ defmodule Mobilizon.Service.RichMedia.Parser do
   alias Mobilizon.Config
   alias Mobilizon.Service.HTTP.RichMediaPreviewClient
   alias Mobilizon.Service.RichMedia.Favicon
+  alias Mobilizon.Service.RichMedia.Parsers.Fallback
   alias Plug.Conn.Utils
   require Logger
 
@@ -73,12 +74,11 @@ defmodule Mobilizon.Service.RichMedia.Parser do
            {:is_html, _response_headers, true} <-
              {:is_html, response_headers, is_html(response_headers)} do
         body
-        |> parse_html()
         |> maybe_parse()
         |> Map.put(:url, url)
         |> maybe_add_favicon()
         |> clean_parsed_data()
-        |> check_parsed_data()
+        |> check_parsed_data(body)
         |> check_remote_picture_path()
       else
         {:is_html, response_headers, false} ->
@@ -192,8 +192,7 @@ defmodule Mobilizon.Service.RichMedia.Parser do
     end
   end
 
-  defp parse_html(html), do: Floki.parse_document!(html)
-
+  @spec maybe_parse(String.t()) :: {:halt, map()} | {:cont, map()}
   defp maybe_parse(html) do
     Enum.reduce_while(parsers(), %{}, fn parser, acc ->
       case parser.parse(html, acc) do
@@ -206,15 +205,25 @@ defmodule Mobilizon.Service.RichMedia.Parser do
     end)
   end
 
-  defp check_parsed_data(%{title: title} = data)
+  defp check_parsed_data(data, html, first_run \\ true)
+
+  defp check_parsed_data(%{title: title} = data, _html, _first_run)
        when is_binary(title) and byte_size(title) > 0 do
     data
   end
 
-  defp check_parsed_data(data) do
+  defp check_parsed_data(data, html, first_run) do
+    # Maybe the first data found is incomplete, pass it through the Fallback parser once again
+    if first_run do
+      {:ok, data} = Fallback.parse(html, data)
+      Logger.debug("check parsed data")
+      Logger.debug(inspect(data))
+      check_parsed_data(data, html, false)
+    else
       Logger.debug("Found metadata was invalid or incomplete: #{inspect(data)}")
       {:error, :invalid_parsed_data}
     end
+  end
 
   defp clean_parsed_data(data) do
     data
@@ -280,25 +289,20 @@ defmodule Mobilizon.Service.RichMedia.Parser do
   @spec check_remote_picture_path(map()) :: map()
   defp check_remote_picture_path(%{image_remote_url: image_remote_url, url: url} = data) do
     Logger.debug("Checking image_remote_url #{image_remote_url}")
-    image_uri = URI.parse(image_remote_url)
-    uri = URI.parse(url)
 
-    image_remote_url =
-      cond do
-        is_nil(image_uri.host) -> "#{uri.scheme}://#{uri.host}#{correct_path(image_remote_url)}"
-        is_nil(image_uri.scheme) -> "#{uri.scheme}:#{image_remote_url}"
-        true -> image_remote_url
-      end
-
-    data = Map.put(data, :image_remote_url, image_remote_url)
+    data = Map.put(data, :image_remote_url, format_url(url, image_remote_url))
     {:ok, data}
   end
 
   defp check_remote_picture_path(data), do: {:ok, data}
 
-  # Sometimes paths have "/" in front, sometimes not
-  defp correct_path("/" <> _ = path), do: path
-  defp correct_path(path), do: "/#{path}"
+  @spec format_url(String.t(), String.t()) :: String.t()
+  defp format_url(url, path) do
+    url
+    |> URI.parse()
+    |> URI.merge(path)
+    |> to_string()
+  end
 
   # Twitter requires a well-know crawler user-agent to show server-rendered data
   defp default_user_agent("https://twitter.com/" <> _) do

lib/service/rich_media/parsers/fallback.ex

@@ -29,11 +29,19 @@ defmodule Mobilizon.Service.RichMedia.Parsers.Fallback do
   end
 
   defp get_page(html, :title) do
-    html |> Floki.find("html head title") |> List.first() |> Floki.text() |> String.trim()
+    html
+    |> Floki.parse_document!()
+    |> Floki.find("html title")
+    |> List.first()
+    |> Floki.text()
+    |> String.trim()
   end
 
   defp get_page(html, :description) do
-    case html |> Floki.find("html head meta[name='description']") |> List.first() do
+    case html
+         |> Floki.parse_document!()
+         |> Floki.find("html meta[name='description']")
+         |> List.first() do
       nil -> ""
       elem -> elem |> Floki.attribute("content") |> List.first() |> String.trim()
     end

lib/service/rich_media/parsers/meta_tags_parser.ex

@@ -36,7 +36,7 @@ defmodule Mobilizon.Service.RichMedia.Parsers.MetaTagsParser do
   end
 
   defp get_elements(html, key_name, prefix) do
-    html |> Floki.find("meta[#{to_string(key_name)}^='#{prefix}:']")
+    html |> Floki.parse_document!() |> Floki.find("meta[#{to_string(key_name)}^='#{prefix}:']")
   end
 
   defp normalize_attributes(html_node, prefix, key_name, value_name, allowed_attributes) do
@@ -83,14 +83,26 @@ defmodule Mobilizon.Service.RichMedia.Parsers.MetaTagsParser do
 
   defp maybe_put_description(meta, _), do: meta
 
+  @spec get_page_title(String.t()) :: String.t()
   defp get_page_title(html) do
-    html |> Floki.find("html head title") |> List.first() |> Floki.text()
+    with {:ok, document} <- Floki.parse_document(html),
+         elem when not is_nil(elem) <- document |> Floki.find("html head title") |> List.first(),
+         title when is_binary(title) <- Floki.text(elem) do
+      title
+    else
+      _ -> ""
+    end
   end
 
+  @spec get_page_description(String.t()) :: String.t()
   defp get_page_description(html) do
-    case html |> Floki.find("html head meta[name='description']") |> List.first() do
-      nil -> ""
-      elem -> Floki.attribute(elem, "content")
+    with {:ok, document} <- Floki.parse_document(html),
+         elem when not is_nil(elem) <-
+           document |> Floki.find("html head meta[name='description']") |> List.first(),
+         description when is_binary(description) <- Floki.attribute(elem, "content") do
+      description
+    else
+      _ -> ""
     end
   end
 end

lib/service/rich_media/parsers/oembed_parser.ex

@@ -32,7 +32,9 @@ defmodule Mobilizon.Service.RichMedia.Parsers.OEmbed do
   end
 
   defp get_discovery_data(html) do
-    html |> Floki.find("link[type='application/json+oembed']")
+    with {:ok, document} <- Floki.parse_document(html) do
+      Floki.find(document, "link[type='application/json+oembed']")
+    end
   end
 
   defp get_oembed_url(nodes) do