Strip alt attribute from unavailable images in user content

closes LS-1590
flag=none

Ideally we'd strip the alt text in the canvas backend, but that turns
out to be very inconvenient. Instead, add a hidden=1 param to the URL
and strip the alt text in the browser. This leaks the alt text to the
crafty user, but is better than nothing.

If this isn't good enough, I can work through the issues for getting
it handled in the backend.

test plan:
  - as a teacher, unpublish an image file that's embedded in
    RCE content.
  - as a student, view the page
  > expect the lock image in place of the real deal
  > expect the alt text to say the image is unavailable

  - as a teacher, schedule student availability so it is not
    currently available
  > as a student, view the page
  > expect the lock image
  > expect the alt text to say the image is unavailable

Change-Id: Id4c7009bb21cc18227d50ce514159fbf73ff2d15
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/253568
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Jeremy Stanley <jeremy@instructure.com>
QA-Review: Robin Kuss <rkuss@instructure.com>
Product-Review: Ed Schiebel <eschiebel@instructure.com>
This commit is contained in:
Ed Schiebel 2020-11-23 14:52:30 -05:00
parent 6b79b18ef8
commit 17316ef4d6
3 changed files with 32 additions and 2 deletions

View File

@ -122,13 +122,18 @@ module UserContent
if user_can_access_attachment?
ProcessedUrl.new(match: match, attachment: attachment, is_public: is_public, in_app: in_app).url
elsif attachment.previewable_media? && match.url.present?
else
begin
uri = URI.parse(match.url)
rescue URI::InvalidURIError
uri = URI.parse(Addressable::URI.escape(match.url))
end
uri.query = (uri.query.to_s.split("&") + ["no_preview=1"]).join("&")
if attachment.previewable_media? && match.url.present?
uri.query = (uri.query.to_s.split("&") + ["no_preview=1"]).join("&")
elsif attachment.locked_for?(user) && attachment.content_type =~ /^image/
# hidden=1 tells the browser to strip the alt attribute for locked files
uri.query = (uri.query.to_s.split("&") + ["hidden=1"]).join("&")
end
uri.to_s
end
end

View File

@ -123,6 +123,13 @@ export function enhanceUserContent() {
} else {
handleWidth()
}
// if the image file is unpublished it's replaced with the lock image
// and canvas adds hidden=1 to the URL.
// we also need to strip the alt text
if (/hidden=1$/.test(img.getAttribute('src'))) {
img.setAttribute('alt', I18n.t('This image is currently unavailable'))
}
})
$this.data('unenhanced_content_html', $this.html())
})

View File

@ -141,6 +141,24 @@ describe UserContent::FilesHandler do
expect(processed_url).to match(/#{attachment.context_type.tableize}/)
end
end
context 'and file is locked' do
let(:attachment) do
attachment_with_context(course, { filename: 'test.jpg', content_type: 'image/jpeg' })
end
it 'returns match_url with hidden=1' do
attachment.locked = true
attachment.save
expect(processed_url).to eq "#{match_url}&hidden=1"
end
it 'returns match_url with hidden=1 if within a locked time window' do
attachment.unlock_at = 1.hour.from_now
attachment.save
expect(processed_url).to eq "#{match_url}&hidden=1"
end
end
end
end