From: Kohei Yoshino <kohei.yoshino@gmail.com>
Date: Fri, 29 Mar 2019 03:22:47 +0000 (-0400)
Subject: Bug 1535190 - Make sure inline attachments are displayed properly
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=b1703737774717cf9f0732d368f69cf2ac51cd64;p=thirdparty%2Fbugzilla.git

Bug 1535190 - Make sure inline attachments are displayed properly
---

diff --git a/Bugzilla/CGI.pm b/Bugzilla/CGI.pm
index 1656a869b..f2a3b7358 100644
--- a/Bugzilla/CGI.pm
+++ b/Bugzilla/CGI.pm
@@ -89,6 +89,7 @@ sub SHOW_BUG_MODAL_CSP {
       'https://www.google-analytics.com'
     ],
     img_src     => ['self', 'https://secure.gravatar.com'],
+    media_src   => ['self'],
     connect_src => [
       'self',
 
@@ -113,6 +114,7 @@ sub SHOW_BUG_MODAL_CSP {
     my $attach_base = Bugzilla->localconfig->{'attachment_base'};
     $attach_base =~ s/\%bugid\%/$bug_id/g;
     push @{$policy{img_src}}, $attach_base;
+    push @{$policy{media_src}}, $attach_base;
   }
 
   return %policy;
diff --git a/extensions/BugModal/template/en/default/bug_modal/activity_stream.html.tmpl b/extensions/BugModal/template/en/default/bug_modal/activity_stream.html.tmpl
index 2661e256a..b66188e66 100644
--- a/extensions/BugModal/template/en/default/bug_modal/activity_stream.html.tmpl
+++ b/extensions/BugModal/template/en/default/bug_modal/activity_stream.html.tmpl
@@ -258,7 +258,7 @@
   [% IF comment.type == constants.CMT_ATTACHMENT_CREATED %]
     [% att = comment.attachment; link = 'attachment.cgi?id=' _ att.id %]
     <div id="att-[% att.id FILTER none %]"
-         class="attachment[% " patch" IF att.ispatch %][% " obsolete" IF att.isobsolete %]"
+         class="attachment[% " patch" IF att.ispatch; " obsolete" IF att.isobsolete; " deleted" IF !att.datasize %]"
          data-id="[% att.id FILTER none %]" itemscope itemtype="http://schema.org/MediaObject"
          [% IF comment.collapsed +%] style="display:none"[% END ~%]>
       <meta itemprop="contentSize" content="[% att.datasize FILTER none %]">
@@ -274,7 +274,7 @@
           <a class="link[% " lightbox" IF att.is_image %]" href="[% link FILTER html %]" itemprop="contentUrl">
         [% END %]
         <span id="att-[% att.id FILTER none %]-description" itemprop="name">[% att.description FILTER html %]</span></a>
-        [% " (obsolete)" IF att.isobsolete %]
+        [% " (obsolete)" IF att.isobsolete; " (deleted)" IF !att.datasize %]
         — <a href="[% link FILTER html %]&amp;action=edit" itemprop="url">Details</a>
         [% IF att.ispatch && Param('splinter_base') %]
           — <a href="[% Bugzilla.splinter_review_url(bug.bug_id, att.id) FILTER none %]">Splinter Review</a>
@@ -288,19 +288,15 @@
     [% comment_tag = 'pre' %]
   [% END %]
 
-  <[% comment_tag FILTER none %] class="comment-text [%= "markdown-body" IF comment.is_markdown %] [%= "bz_private" IF comment.is_private %]"
-                     id="ct-[% comment.count FILTER none %]"
-                     data-comment-id="[% comment.id FILTER none %]"
-                     [% IF comment.is_markdown +%] data-ismarkdown="true" [% END ~%]
-                     [% IF comment.collapsed +%] style="display:none"[% END ~%]
-  >[% FILTER collapse %]
-    [% IF comment.is_about_attachment && comment.attachment.is_image ~%]
-      <a href="attachment.cgi?id=[% comment.attachment.id FILTER none %]"
-        title="[% comment.attachment.description FILTER html %]"
-        class="lightbox lightbox-icon [%= "markdown" IF comment_tag == 'div' %]"><img src="extensions/BugModal/web/image.png" width="16" height="16"></a>
-    [% END %]
+  [% IF comment.body %]
+    <[% comment_tag FILTER none %]
+        class="comment-text [%= "markdown-body" IF comment.is_markdown %] [%= "bz_private" IF comment.is_private %]"
+        id="ct-[% comment.count FILTER none %]" data-comment-id="[% comment.id FILTER none %]"
+        [% IF comment.is_markdown +%] data-ismarkdown="true" [% END ~%]
+        [% IF comment.collapsed +%] style="display:none"[% END ~%]>
+      [%~ comment.body_full({ exclude_attachment => 1 }) FILTER renderMarkdown(bug, comment) ~%]
+    </[% comment_tag FILTER none %]>
   [% END %]
-  [%~ comment.body_full({ exclude_attachment => 1 }) FILTER renderMarkdown(bug, comment) ~%]</[% comment_tag FILTER none %]>
 [% END %]
 
 [%
diff --git a/extensions/BugModal/web/bug_modal.css b/extensions/BugModal/web/bug_modal.css
index 242b92fa0..79e5deab2 100644
--- a/extensions/BugModal/web/bug_modal.css
+++ b/extensions/BugModal/web/bug_modal.css
@@ -754,12 +754,8 @@ h3.change-name {
 
 .change-set .attachment {
     border-top: 1px solid #ddd;
-    background-color: #FFF;
-}
-
-.change-set .attachment {
-    background-color: #FFF;
     padding: 8px;
+    background-color: #FFF;
 }
 
 .change-set .attachment .label {
@@ -783,6 +779,13 @@ h3.change-name {
     color: #333;
 }
 
+.change-set .attachment button.outer {
+  padding: 0;
+  font-weight: normal;
+  box-shadow: none;
+  transition: none;
+}
+
 .change-set .attachment .lightbox {
     cursor: zoom-in;
 }
@@ -838,6 +841,10 @@ h3.change-name {
     padding-top: 0;
 }
 
+.change-set .attachment ~ .comment-text:empty {
+    padding-bottom: 0;
+}
+
 /* add comment */
 
 #add-comment {
diff --git a/extensions/BugModal/web/comments.js b/extensions/BugModal/web/comments.js
index 3051fea2e..8ce5eaa6c 100644
--- a/extensions/BugModal/web/comments.js
+++ b/extensions/BugModal/web/comments.js
@@ -515,10 +515,9 @@ Bugzilla.BugModal.Comments = class Comments {
   }
 
   /**
-   * Prepare to show image and text attachments inline if possible. For a better performance, this functionality uses
-   * the Intersection Observer API to show attachments when the associated comment goes into the viewport, when the page
-   * is scrolled down or the collapsed comment is expanded. This also utilizes the Network Information API to save
-   * bandwidth over cellular networks.
+   * Prepare to show image, media and text attachments inline if possible. For a better performance, this functionality
+   * uses the Intersection Observer API to show attachments when the associated comment goes into the viewport, when the
+   * page is scrolled down or the collapsed comment is expanded. This also utilizes the Network Information API to save
    * @see https://developer.mozilla.org/en-US/docs/Web/API/Intersection_Observer_API
    * @see https://developer.mozilla.org/en-US/docs/Web/API/Network_Information_API
    */
@@ -538,8 +537,9 @@ Bugzilla.BugModal.Comments = class Comments {
       }
     }), { root: document.querySelector('#bugzilla-body') });
 
-    // Show only non-obsolete attachments
-    document.querySelectorAll('.change-set .attachment:not(.obsolete)').forEach($att => observer.observe($att));
+    // Show attachments except for obsolete or deleted ones
+    document.querySelectorAll('.change-set .attachment:not(.obsolete):not(.deleted)')
+      .forEach($att => observer.observe($att));
   }
 
   /**
@@ -581,32 +581,34 @@ Bugzilla.BugModal.Comments = class Comments {
     }
 
     // Detect text (code from attachment.js)
-    const is_patch = !!name.match(/\.(?:diff|patch)$/) || !!type.match(/^text\/x-(?:diff|patch)$/);
+    const is_patch = $att.matches('.patch');
     const is_markdown = !!name.match(/\.(?:md|mkdn?|mdown|markdown)$/);
     const is_source = !!name.match(/\.(?:cpp|es|h|js|json|rs|rst|sh|toml|ts|tsx|xml|yaml|yml)$/);
-    const is_text = type.startsWith('text/') || is_patch || is_markdown || is_source;
+    const is_text = type.match(/^text\/(?!x-).+$/) || is_patch || is_markdown || is_source;
 
     // Show text smaller than 2 MB
     if (is_text && size < max_size) {
       // Load text body
-      try {
-        const response = await fetch(`/attachment.cgi?id=${id}`, { credentials: 'same-origin' });
-
-        if (!response.ok) {
-          throw new Error();
+      bugzilla_ajax({ url: `${BUGZILLA.config.basepath}rest/bug/attachment/${id}?include_fields=data` }, data => {
+        if (data.error) {
+          return;
         }
 
-        const text = await response.text();
+        const text = decodeURIComponent(escape(atob(data.attachments[id].data)));
         const lang = is_patch ? 'diff' : type.match(/\w+$/)[0];
 
         $att.insertAdjacentHTML('beforeend', `
-          <a href="${link}" title="${name}" class="outer">
-          <pre class="language-${lang}" role="img" itemprop="text">${text}</pre></a>`);
+          <button type="button" role="link" title="${name}" class="outer">
+          <pre class="language-${lang}" role="img" itemprop="text">${text.htmlEncode()}</pre></button>`);
+
+        // Make the button work as a link. It cannot be `<a>` because Prism Autolinker plugin may add links to `<pre>`
+        $att.querySelector('[role="link"]').addEventListener('click', () => location.href = link);
 
         if (Prism) {
           Prism.highlightElement($att.querySelector('pre'));
+          $att.querySelectorAll('pre a').forEach($a => $a.tabIndex = -1);
         }
-      } catch (ex) {}
+      });
     }
   }
 };
diff --git a/js/global.js b/js/global.js
index 8e18f310b..691229caf 100644
--- a/js/global.js
+++ b/js/global.js
@@ -160,8 +160,10 @@ function display_value(field, value) {
 function bugzilla_ajax(request, done_fn, error_fn) {
     $('#xhr-error').hide('');
     $('#xhr-error').html('');
-    request.url += (request.url.match('\\?') ? '&' : '?') +
-        'Bugzilla_api_token=' + encodeURIComponent(BUGZILLA.api_token);
+    if (BUGZILLA.api_token) {
+        request.url += (request.url.match('\\?') ? '&' : '?') +
+            'Bugzilla_api_token=' + encodeURIComponent(BUGZILLA.api_token);
+    }
     if (request.type != 'GET') {
         request.contentType = 'application/json';
         request.processData = false;
diff --git a/skins/standard/global.css b/skins/standard/global.css
index d58abbdb0..69dfe8715 100644
--- a/skins/standard/global.css
+++ b/skins/standard/global.css
@@ -2139,10 +2139,9 @@ pre.comment-text {
     font-family: "Droid Sans Mono",Menlo,Monaco,"Courier New",monospace;
     background: #fff;
     color: #222;
-    margin: 1px 0 0 0;
+    margin: 0;
     overflow: auto;
     padding: 8px;
-    border-top: 1px solid #ddd;
 }
 
 .comment-text span.quote, .comment-text span.quote_wrapped {